Merge pull request #425 from crytic/dev-upgradeable-support

Improve report on upgradeable proxy codebase

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

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
         """

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: