From 6848bca14d3b61b2c45b7af55c188a50b4950475 Mon Sep 17 00:00:00 2001 From: Josselin Date: Mon, 30 Mar 2020 16:49:37 +0200 Subject: [PATCH] - Add function.all_nodes function - Improve is_upgradeable_proxy by using all_nodes - Improve controlled-delegatecall detector by using is_upgradeable_proxy (fix #420) --- slither/core/declarations/contract.py | 2 +- slither/core/declarations/function.py | 8 ++++++++ slither/detectors/statements/controlled_delegatecall.py | 9 ++++++--- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py index 3eb8164f6..e9a430289 100644 --- a/slither/core/declarations/contract.py +++ b/slither/core/declarations/contract.py @@ -863,7 +863,7 @@ class Contract(ChildSlither, SourceMapping): self._is_upgradeable_proxy = False for f in self.functions: if f.is_fallback: - for node in f.nodes: + for node in f.all_nodes(): for ir in node.irs: if isinstance(ir, LowLevelCall) and ir.function_name == 'delegatecall': self._is_upgradeable_proxy = True diff --git a/slither/core/declarations/function.py b/slither/core/declarations/function.py index b8a293c14..4a5c9aace 100644 --- a/slither/core/declarations/function.py +++ b/slither/core/declarations/function.py @@ -121,6 +121,7 @@ class Function(ChildContract, ChildInheritance, SourceMapping): self._all_solidity_variables_read = None self._all_state_variables_written = None self._all_slithir_variables = None + self._all_nodes = None self._all_conditional_state_variables_read = None self._all_conditional_state_variables_read_with_loop = None self._all_conditional_solidity_variables_read = None @@ -833,6 +834,13 @@ class Function(ChildContract, ChildInheritance, SourceMapping): lambda x: x.slithir_variable) return self._all_slithir_variables + def all_nodes(self): + """ recursive version of nodes + """ + if self._all_nodes is None: + self._all_nodes = self._explore_functions(lambda x: x.nodes) + return self._all_nodes + def all_expressions(self): """ recursive version of variables_read """ diff --git a/slither/detectors/statements/controlled_delegatecall.py b/slither/detectors/statements/controlled_delegatecall.py index 987757847..334c6cd41 100644 --- a/slither/detectors/statements/controlled_delegatecall.py +++ b/slither/detectors/statements/controlled_delegatecall.py @@ -34,16 +34,19 @@ Bob calls `delegate` and delegates the execution to its malicious contract. As a for node in function.nodes: for ir in node.irs: if isinstance(ir, LowLevelCall) and ir.function_name in ['delegatecall', 'callcode']: - if is_tainted(ir.destination, function.contract): + if is_tainted(ir.destination, + function.contract): ret.append(node) return ret def _detect(self): results = [] - for contract in self.slither.contracts: + for contract in self.slither.contracts_derived: for f in contract.functions: - if f.contract_declarer != contract: + # If its an upgradeable proxy, do not report protected function + # As functions to upgrades the destination lead to too many FPs + if contract.is_upgradeable_proxy and f.is_protected(): continue nodes = self.controlled_delegatecall(f) if nodes: