From 46b26678e13ba5bc0d915e2d91a6af0eb84f7160 Mon Sep 17 00:00:00 2001 From: Nikhil Parasaram Date: Fri, 25 Mar 2022 09:37:49 +0000 Subject: [PATCH] Fix caching of issues across modules (#1610) * Fix caching of issues across modules * Fix caching of issues across modules --- mythril/analysis/module/base.py | 34 +++++++++++++++---- .../analysis/module/modules/arbitrary_jump.py | 4 +-- .../module/modules/arbitrary_write.py | 2 -- .../analysis/module/modules/delegatecall.py | 2 -- .../module/modules/dependence_on_origin.py | 9 ++--- .../modules/dependence_on_predictable_vars.py | 9 ++--- .../analysis/module/modules/ether_thief.py | 2 -- mythril/analysis/module/modules/exceptions.py | 12 ++----- .../analysis/module/modules/external_calls.py | 1 + mythril/analysis/module/modules/integer.py | 5 --- .../analysis/module/modules/multiple_sends.py | 7 +--- .../modules/state_change_external_calls.py | 2 -- mythril/analysis/module/modules/suicide.py | 7 +--- .../module/modules/unchecked_retval.py | 9 ++--- .../module/modules/user_assertions.py | 5 +-- mythril/analysis/potential_issues.py | 1 + mythril/support/support_utils.py | 2 ++ 17 files changed, 43 insertions(+), 70 deletions(-) diff --git a/mythril/analysis/module/base.py b/mythril/analysis/module/base.py index 0459cf2a..db876ef2 100644 --- a/mythril/analysis/module/base.py +++ b/mythril/analysis/module/base.py @@ -8,7 +8,7 @@ from typing import List, Set, Optional, Tuple, Union from mythril.analysis.report import Issue from mythril.laser.ethereum.state.global_state import GlobalState - +from mythril.support.support_utils import get_code_hash from abc import ABC, abstractmethod from enum import Enum @@ -45,18 +45,27 @@ class DetectionModule(ABC): name = "Detection Module Name / Title" swc_id = "SWC-000" description = "Detection module description" - entry_point = EntryPoint.CALLBACK # type: EntryPoint - pre_hooks = [] # type: List[str] - post_hooks = [] # type: List[str] + entry_point: EntryPoint = EntryPoint.CALLBACK + pre_hooks: List[str] = [] + post_hooks: List[str] = [] def __init__(self) -> None: - self.issues = [] # type: List[Issue] - self.cache = set() # type: Set[Optional[Union[int, Tuple[int, str]]]] + self.issues: List[Issue] = [] + self.cache: Set[Tuple[int, str]] = set() def reset_module(self): """Resets the storage of this module""" self.issues = [] + def update_cache(self, issues=None): + """ + Updates cache with param issues, updates against self.issues, if the param is None + - issues: The issues used to update the cache + """ + issues = issues or self.issues + for issue in self.issues: + self.cache.add((issue.address, issue.bytecode_hash)) + def execute(self, target: GlobalState) -> Optional[List[Issue]]: """The entry point for execution, which is being called by Mythril. @@ -66,9 +75,20 @@ class DetectionModule(ABC): log.debug("Entering analysis module: {}".format(self.__class__.__name__)) - result = self._execute(target) + if ( + target.get_current_instruction()["address"], + get_code_hash(target.environment.code.bytecode), + ) in self.cache: + log.info( + f"Issue in cache for the analysis module: {self.__class__.__name__}, address: {target.get_current_instruction()['address']}" + ) + return [] + result = self._execute(target) log.debug("Exiting analysis module: {}".format(self.__class__.__name__)) + if result: + self.update_cache(result) + self.issues += result return result diff --git a/mythril/analysis/module/modules/arbitrary_jump.py b/mythril/analysis/module/modules/arbitrary_jump.py index 61d14b99..df4cfaae 100644 --- a/mythril/analysis/module/modules/arbitrary_jump.py +++ b/mythril/analysis/module/modules/arbitrary_jump.py @@ -35,9 +35,7 @@ class ArbitraryJump(DetectionModule): :param state: :return: """ - if state.get_current_instruction()["address"] in self.cache: - return - self.issues.extend(self._analyze_state(state)) + return self._analyze_state(state) @staticmethod def _analyze_state(state): diff --git a/mythril/analysis/module/modules/arbitrary_write.py b/mythril/analysis/module/modules/arbitrary_write.py index c6523213..edceaf70 100644 --- a/mythril/analysis/module/modules/arbitrary_write.py +++ b/mythril/analysis/module/modules/arbitrary_write.py @@ -40,8 +40,6 @@ class ArbitraryStorage(DetectionModule): :param state: :return: """ - if state.get_current_instruction()["address"] in self.cache: - return potential_issues = self._analyze_state(state) annotation = get_potential_issues_annotation(state) diff --git a/mythril/analysis/module/modules/delegatecall.py b/mythril/analysis/module/modules/delegatecall.py index a0de4ac1..7e7c2829 100644 --- a/mythril/analysis/module/modules/delegatecall.py +++ b/mythril/analysis/module/modules/delegatecall.py @@ -34,8 +34,6 @@ class ArbitraryDelegateCall(DetectionModule): :param state: :return: """ - if state.get_current_instruction()["address"] in self.cache: - return potential_issues = self._analyze_state(state) annotation = get_potential_issues_annotation(state) diff --git a/mythril/analysis/module/modules/dependence_on_origin.py b/mythril/analysis/module/modules/dependence_on_origin.py index 4f43cc64..5e8e2c4e 100644 --- a/mythril/analysis/module/modules/dependence_on_origin.py +++ b/mythril/analysis/module/modules/dependence_on_origin.py @@ -32,18 +32,13 @@ class TxOrigin(DetectionModule): pre_hooks = ["JUMPI"] post_hooks = ["ORIGIN"] - def _execute(self, state: GlobalState) -> None: + def _execute(self, state: GlobalState) -> List[Issue]: """ :param state: :return: """ - if state.get_current_instruction()["address"] in self.cache: - return - issues = self._analyze_state(state) - for issue in issues: - self.cache.add(issue.address) - self.issues.extend(issues) + return self._analyze_state(state) @staticmethod def _analyze_state(state: GlobalState) -> List[Issue]: diff --git a/mythril/analysis/module/modules/dependence_on_predictable_vars.py b/mythril/analysis/module/modules/dependence_on_predictable_vars.py index 4fabc337..8702f865 100644 --- a/mythril/analysis/module/modules/dependence_on_predictable_vars.py +++ b/mythril/analysis/module/modules/dependence_on_predictable_vars.py @@ -46,18 +46,13 @@ class PredictableVariables(DetectionModule): pre_hooks = ["JUMPI", "BLOCKHASH"] post_hooks = ["BLOCKHASH"] + predictable_ops - def _execute(self, state: GlobalState) -> None: + def _execute(self, state: GlobalState) -> List[Issue]: """ :param state: :return: """ - if state.get_current_instruction()["address"] in self.cache: - return - issues = self._analyze_state(state) - for issue in issues: - self.cache.add(issue.address) - self.issues.extend(issues) + return self._analyze_state(state) @staticmethod def _analyze_state(state: GlobalState) -> List[Issue]: diff --git a/mythril/analysis/module/modules/ether_thief.py b/mythril/analysis/module/modules/ether_thief.py index a0a3cf97..0cc6436d 100644 --- a/mythril/analysis/module/modules/ether_thief.py +++ b/mythril/analysis/module/modules/ether_thief.py @@ -46,8 +46,6 @@ class EtherThief(DetectionModule): :param state: :return: """ - if state.get_current_instruction()["address"] in self.cache: - return potential_issues = self._analyze_state(state) annotation = get_potential_issues_annotation(state) diff --git a/mythril/analysis/module/modules/exceptions.py b/mythril/analysis/module/modules/exceptions.py index 4fab643b..d2c90e20 100644 --- a/mythril/analysis/module/modules/exceptions.py +++ b/mythril/analysis/module/modules/exceptions.py @@ -37,21 +37,13 @@ class Exceptions(DetectionModule): entry_point = EntryPoint.CALLBACK pre_hooks = ["INVALID", "JUMP", "REVERT"] - def _execute(self, state: GlobalState) -> None: + def _execute(self, state: GlobalState) -> List[Issue]: """ :param state: :return: """ - if ( - state.get_current_instruction()["address"], - state.environment.active_function_name, - ) in self.cache: - return - issues = self._analyze_state(state) - for issue in issues: - self.cache.add((issue.address, issue.function)) - self.issues.extend(issues) + return self._analyze_state(state) @staticmethod def _analyze_state(state) -> List[Issue]: diff --git a/mythril/analysis/module/modules/external_calls.py b/mythril/analysis/module/modules/external_calls.py index c1471eb0..1e709405 100644 --- a/mythril/analysis/module/modules/external_calls.py +++ b/mythril/analysis/module/modules/external_calls.py @@ -59,6 +59,7 @@ class ExternalCalls(DetectionModule): :param state: :return: """ + potential_issues = self._analyze_state(state) annotation = get_potential_issues_annotation(state) diff --git a/mythril/analysis/module/modules/integer.py b/mythril/analysis/module/modules/integer.py index c5f994c7..b71fcdec 100644 --- a/mythril/analysis/module/modules/integer.py +++ b/mythril/analysis/module/modules/integer.py @@ -109,11 +109,6 @@ class IntegerArithmetics(DetectionModule): :return: Found issues """ - address = _get_address_from_state(state) - - if address in self.cache: - return - opcode = state.get_current_instruction()["opcode"] funcs = { diff --git a/mythril/analysis/module/modules/multiple_sends.py b/mythril/analysis/module/modules/multiple_sends.py index bc9e544d..2dc2bf1f 100644 --- a/mythril/analysis/module/modules/multiple_sends.py +++ b/mythril/analysis/module/modules/multiple_sends.py @@ -34,12 +34,7 @@ class MultipleSends(DetectionModule): pre_hooks = ["CALL", "DELEGATECALL", "STATICCALL", "CALLCODE", "RETURN", "STOP"] def _execute(self, state: GlobalState) -> None: - if state.get_current_instruction()["address"] in self.cache: - return - issues = self._analyze_state(state) - for issue in issues: - self.cache.add(issue.address) - self.issues.extend(issues) + return self._analyze_state(state) @staticmethod def _analyze_state(state: GlobalState): diff --git a/mythril/analysis/module/modules/state_change_external_calls.py b/mythril/analysis/module/modules/state_change_external_calls.py index c28c6f9c..aaa523ec 100644 --- a/mythril/analysis/module/modules/state_change_external_calls.py +++ b/mythril/analysis/module/modules/state_change_external_calls.py @@ -111,8 +111,6 @@ class StateChangeAfterCall(DetectionModule): pre_hooks = CALL_LIST + STATE_READ_WRITE_LIST def _execute(self, state: GlobalState) -> None: - if state.get_current_instruction()["address"] in self.cache: - return issues = self._analyze_state(state) annotation = get_potential_issues_annotation(state) diff --git a/mythril/analysis/module/modules/suicide.py b/mythril/analysis/module/modules/suicide.py index ee8d0bdf..1c834e69 100644 --- a/mythril/analysis/module/modules/suicide.py +++ b/mythril/analysis/module/modules/suicide.py @@ -47,12 +47,7 @@ class AccidentallyKillable(DetectionModule): :param state: :return: """ - if state.get_current_instruction()["address"] in self.cache: - return - issues = self._analyze_state(state) - for issue in issues: - self.cache.add(issue.address) - self.issues.extend(issues) + return self._analyze_state(state) @staticmethod def _analyze_state(state): diff --git a/mythril/analysis/module/modules/unchecked_retval.py b/mythril/analysis/module/modules/unchecked_retval.py index 2ad3a37a..2d9cc555 100644 --- a/mythril/analysis/module/modules/unchecked_retval.py +++ b/mythril/analysis/module/modules/unchecked_retval.py @@ -52,18 +52,13 @@ class UncheckedRetval(DetectionModule): pre_hooks = ["STOP", "RETURN"] post_hooks = ["CALL", "DELEGATECALL", "STATICCALL", "CALLCODE"] - def _execute(self, state: GlobalState) -> None: + def _execute(self, state: GlobalState) -> List[Issue]: """ :param state: :return: """ - if state.get_current_instruction()["address"] in self.cache: - return - issues = self._analyze_state(state) - for issue in issues: - self.cache.add(issue.address) - self.issues.extend(issues) + return self._analyze_state(state) def _analyze_state(self, state: GlobalState) -> list: instruction = state.get_current_instruction() diff --git a/mythril/analysis/module/modules/user_assertions.py b/mythril/analysis/module/modules/user_assertions.py index 62c042d6..d5bcf645 100644 --- a/mythril/analysis/module/modules/user_assertions.py +++ b/mythril/analysis/module/modules/user_assertions.py @@ -43,10 +43,7 @@ class UserAssertions(DetectionModule): :return: """ - issues = self._analyze_state(state) - for issue in issues: - self.cache.add(issue.address) - self.issues.extend(issues) + return self._analyze_state(state) def _analyze_state(self, state: GlobalState): """ diff --git a/mythril/analysis/potential_issues.py b/mythril/analysis/potential_issues.py index 1965edf6..ca2fb4ea 100644 --- a/mythril/analysis/potential_issues.py +++ b/mythril/analysis/potential_issues.py @@ -111,4 +111,5 @@ def check_potential_issues(state: GlobalState) -> None: transaction_sequence=transaction_sequence, ) ) + potential_issue.detector.update_cache() annotation.potential_issues = unsat_potential_issues diff --git a/mythril/support/support_utils.py b/mythril/support/support_utils.py index 6ca94184..ec4da07c 100644 --- a/mythril/support/support_utils.py +++ b/mythril/support/support_utils.py @@ -1,4 +1,5 @@ """This module contains utility functions for the Mythril support package.""" +from functools import lru_cache from typing import Dict import logging import _pysha3 @@ -26,6 +27,7 @@ class Singleton(type): return cls._instances[cls] +@lru_cache(maxsize=2 ** 10) def get_code_hash(code) -> str: """ :param code: bytecode