Merge pull request #2696 from crytic/dev-detectors-add-callstack

Add calls stack information to detectors

Author: smonicas

Diff for: slither/detectors/statements/calls_in_loop.py

@@ -1,4 +1,4 @@
-from typing import List, Optional
+from typing import List, Optional, Tuple
 from slither.core.cfg.node import NodeType, Node
 from slither.detectors.abstract_detector import (
     AbstractDetector,
@@ -16,18 +16,24 @@
     InternalCall,
 )
 
+Result = List[Tuple[Node, List[str]]]
 
-def detect_call_in_loop(contract: Contract) -> List[Node]:
-    ret: List[Node] = []
+
+def detect_call_in_loop(contract: Contract) -> Result:
+    ret: Result = []
     for f in contract.functions_entry_points:
         if f.is_implemented:
-            call_in_loop(f.entry_point, 0, [], ret)
+            call_in_loop(f.entry_point, 0, [], [], ret)
 
     return ret
 
 
 def call_in_loop(
-    node: Optional[Node], in_loop_counter: int, visited: List[Node], ret: List[Node]
+    node: Optional[Node],
+    in_loop_counter: int,
+    visited: List[Node],
+    calls_stack: List[str],
+    ret: Result,
 ) -> None:
     if node is None:
         return
@@ -41,18 +47,19 @@ def call_in_loop(
     elif node.type == NodeType.ENDLOOP:
         in_loop_counter -= 1
 
-    if in_loop_counter > 0:
-        for ir in node.all_slithir_operations():
-            if isinstance(ir, (LowLevelCall, HighLevelCall, Send, Transfer)):
-                if isinstance(ir, LibraryCall):
-                    continue
-                ret.append(ir.node)
-            if isinstance(ir, (InternalCall)):
-                assert ir.function
-                call_in_loop(ir.function.entry_point, in_loop_counter, visited, ret)
+    for ir in node.irs:
+        if isinstance(ir, (LowLevelCall, HighLevelCall, Send, Transfer)) and in_loop_counter > 0:
+            if isinstance(ir, LibraryCall):
+                continue
+            ret.append((ir.node, calls_stack.copy()))
+        if isinstance(ir, (InternalCall)):
+            assert ir.function
+            calls_stack.append(node.function.canonical_name)
+            call_in_loop(ir.function.entry_point, in_loop_counter, visited, calls_stack, ret)
+            calls_stack.pop()
 
     for son in node.sons:
-        call_in_loop(son, in_loop_counter, visited, ret)
+        call_in_loop(son, in_loop_counter, visited, calls_stack, ret)
 
 
 class MultipleCallsInLoop(AbstractDetector):
@@ -96,10 +103,16 @@ def _detect(self) -> List[Output]:
         results: List[Output] = []
         for c in self.compilation_unit.contracts_derived:
             values = detect_call_in_loop(c)
-            for node in values:
+            for node, calls_stack in values:
                 func = node.function
 
                 info: DETECTOR_INFO = [func, " has external calls inside a loop: ", node, "\n"]
+
+                if len(calls_stack) > 0:
+                    info.append("\tCalls stack containing the loop:\n")
+                    for call in calls_stack:
+                        info.extend(["\t\t", call, "\n"])
+
                 res = self.generate_result(info)
                 results.append(res)
 

Diff for: slither/detectors/statements/costly_operations_in_loop.py

@@ -1,4 +1,4 @@
-from typing import List, Optional
+from typing import List, Optional, Tuple
 from slither.core.cfg.node import NodeType, Node
 from slither.detectors.abstract_detector import (
     AbstractDetector,
@@ -10,18 +10,24 @@
 from slither.slithir.operations import InternalCall, OperationWithLValue
 from slither.core.variables.state_variable import StateVariable
 
+Result = List[Tuple[Node, List[str]]]
 
-def detect_costly_operations_in_loop(contract: Contract) -> List[Node]:
-    ret: List[Node] = []
+
+def detect_costly_operations_in_loop(contract: Contract) -> Result:
+    ret: Result = []
     for f in contract.functions_entry_points:
         if f.is_implemented:
-            costly_operations_in_loop(f.entry_point, 0, [], ret)
+            costly_operations_in_loop(f.entry_point, 0, [], [], ret)
 
     return ret
 
 
 def costly_operations_in_loop(
-    node: Optional[Node], in_loop_counter: int, visited: List[Node], ret: List[Node]
+    node: Optional[Node],
+    in_loop_counter: int,
+    visited: List[Node],
+    calls_stack: List[str],
+    ret: Result,
 ) -> None:
 
     if node is None:
@@ -38,16 +44,20 @@ def costly_operations_in_loop(
         in_loop_counter -= 1
 
     if in_loop_counter > 0:
-        for ir in node.all_slithir_operations():
+        for ir in node.irs:
             # Ignore Array/Mapping/Struct types for now
             if isinstance(ir, OperationWithLValue) and isinstance(ir.lvalue, StateVariable):
-                ret.append(ir.node)
+                ret.append((ir.node, calls_stack.copy()))
                 break
             if isinstance(ir, (InternalCall)) and ir.function:
-                costly_operations_in_loop(ir.function.entry_point, in_loop_counter, visited, ret)
+                calls_stack.append(node.function.canonical_name)
+                costly_operations_in_loop(
+                    ir.function.entry_point, in_loop_counter, visited, calls_stack, ret
+                )
+                calls_stack.pop()
 
     for son in node.sons:
-        costly_operations_in_loop(son, in_loop_counter, visited, ret)
+        costly_operations_in_loop(son, in_loop_counter, visited, calls_stack, ret)
 
 
 class CostlyOperationsInLoop(AbstractDetector):
@@ -100,10 +110,16 @@ def _detect(self) -> List[Output]:
         results: List[Output] = []
         for c in self.compilation_unit.contracts_derived:
             values = detect_costly_operations_in_loop(c)
-            for node in values:
+            for node, calls_stack in values:
                 func = node.function
                 info: DETECTOR_INFO = [func, " has costly operations inside a loop:\n"]
                 info += ["\t- ", node, "\n"]
+
+                if len(calls_stack) > 0:
+                    info.append("\tCalls stack containing the loop:\n")
+                    for call in calls_stack:
+                        info.extend(["\t\t", call, "\n"])
+
                 res = self.generate_result(info)
                 results.append(res)
 

Diff for: slither/detectors/statements/delegatecall_in_loop.py

@@ -1,4 +1,4 @@
-from typing import List, Optional
+from typing import List, Optional, Tuple
 from slither.core.cfg.node import NodeType, Node
 from slither.detectors.abstract_detector import (
     AbstractDetector,
@@ -9,17 +9,23 @@
 from slither.core.declarations import Contract
 from slither.utils.output import Output
 
+Result = List[Tuple[Node, List[str]]]
 
-def detect_delegatecall_in_loop(contract: Contract) -> List[Node]:
-    results: List[Node] = []
+
+def detect_delegatecall_in_loop(contract: Contract) -> Result:
+    results: Result = []
     for f in contract.functions_entry_points:
         if f.is_implemented and f.payable:
-            delegatecall_in_loop(f.entry_point, 0, [], results)
+            delegatecall_in_loop(f.entry_point, 0, [], [], results)
     return results
 
 
 def delegatecall_in_loop(
-    node: Optional[Node], in_loop_counter: int, visited: List[Node], results: List[Node]
+    node: Optional[Node],
+    in_loop_counter: int,
+    visited: List[Node],
+    calls_stack: List[str],
+    results: Result,
 ) -> None:
 
     if node is None:
@@ -35,18 +41,22 @@ def delegatecall_in_loop(
     elif node.type == NodeType.ENDLOOP:
         in_loop_counter -= 1
 
-    for ir in node.all_slithir_operations():
+    for ir in node.irs:
         if (
             in_loop_counter > 0
             and isinstance(ir, (LowLevelCall))
             and ir.function_name == "delegatecall"
         ):
-            results.append(ir.node)
+            results.append((ir.node, calls_stack.copy()))
         if isinstance(ir, (InternalCall)) and ir.function:
-            delegatecall_in_loop(ir.function.entry_point, in_loop_counter, visited, results)
+            calls_stack.append(node.function.canonical_name)
+            delegatecall_in_loop(
+                ir.function.entry_point, in_loop_counter, visited, calls_stack, results
+            )
+            calls_stack.pop()
 
     for son in node.sons:
-        delegatecall_in_loop(son, in_loop_counter, visited, results)
+        delegatecall_in_loop(son, in_loop_counter, visited, calls_stack, results)
 
 
 class DelegatecallInLoop(AbstractDetector):
@@ -95,7 +105,7 @@ def _detect(self) -> List[Output]:
         results: List[Output] = []
         for c in self.compilation_unit.contracts_derived:
             values = detect_delegatecall_in_loop(c)
-            for node in values:
+            for node, calls_stack in values:
                 func = node.function
 
                 info: DETECTOR_INFO = [
@@ -104,6 +114,12 @@ def _detect(self) -> List[Output]:
                     node,
                     "\n",
                 ]
+
+                if len(calls_stack) > 0:
+                    info.append("\tCalls stack containing the loop:\n")
+                    for call in calls_stack:
+                        info.extend(["\t\t", call, "\n"])
+
                 res = self.generate_result(info)
                 results.append(res)
 

Diff for: slither/detectors/statements/msg_value_in_loop.py

@@ -1,4 +1,4 @@
-from typing import List, Optional
+from typing import List, Optional, Tuple
 from slither.core.cfg.node import NodeType, Node
 from slither.detectors.abstract_detector import (
     AbstractDetector,
@@ -12,17 +12,23 @@
 from slither.core.variables import Variable
 from slither.core.expressions.literal import Literal
 
+Result = List[Tuple[Node, List[str]]]
 
-def detect_msg_value_in_loop(contract: Contract) -> List[Node]:
-    results: List[Node] = []
+
+def detect_msg_value_in_loop(contract: Contract) -> Result:
+    results: Result = []
     for f in contract.functions_entry_points:
         if f.is_implemented and f.payable:
-            msg_value_in_loop(f.entry_point, 0, [], results)
+            msg_value_in_loop(f.entry_point, 0, [], [], results)
     return results
 
 
 def msg_value_in_loop(
-    node: Optional[Node], in_loop_counter: int, visited: List[Node], results: List[Node]
+    node: Optional[Node],
+    in_loop_counter: int,
+    visited: List[Node],
+    calls_stack: List[str],
+    results: Result,
 ) -> None:
 
     if node is None:
@@ -38,7 +44,7 @@ def msg_value_in_loop(
     elif node.type == NodeType.ENDLOOP:
         in_loop_counter -= 1
 
-    for ir in node.all_slithir_operations():
+    for ir in node.irs:
         if in_loop_counter > 0 and SolidityVariableComposed("msg.value") in ir.read:
             # If we find a conditional expression with msg.value and is compared to 0 we don't report it
             if ir.node.is_conditional() and SolidityVariableComposed("msg.value") in ir.read:
@@ -55,12 +61,16 @@ def msg_value_in_loop(
                     and str(compared_to.expression.value) == "0"
                 ):
                     continue
-            results.append(ir.node)
+            results.append((ir.node, calls_stack.copy()))
         if isinstance(ir, (InternalCall)):
-            msg_value_in_loop(ir.function.entry_point, in_loop_counter, visited, results)
+            calls_stack.append(node.function.canonical_name)
+            msg_value_in_loop(
+                ir.function.entry_point, in_loop_counter, visited, calls_stack, results
+            )
+            calls_stack.pop()
 
     for son in node.sons:
-        msg_value_in_loop(son, in_loop_counter, visited, results)
+        msg_value_in_loop(son, in_loop_counter, visited, calls_stack, results)
 
 
 class MsgValueInLoop(AbstractDetector):
@@ -105,10 +115,16 @@ def _detect(self) -> List[Output]:
         results: List[Output] = []
         for c in self.compilation_unit.contracts_derived:
             values = detect_msg_value_in_loop(c)
-            for node in values:
+            for node, calls_stack in values:
                 func = node.function
 
                 info: DETECTOR_INFO = [func, " use msg.value in a loop: ", node, "\n"]
+
+                if len(calls_stack) > 0:
+                    info.append("\tCalls stack containing the loop:\n")
+                    for call in calls_stack:
+                        info.extend(["\t\t", call, "\n"])
+
                 res = self.generate_result(info)
                 results.append(res)
 

Diff for: tests/e2e/detectors/snapshots/detectors__detector_CostlyOperationsInLoop_0_4_25_multiple_costly_operations_in_loop_sol__0.txt

@@ -1,12 +1,14 @@
 CostlyOperationsInLoopBase.bad_base() (tests/e2e/detectors/test_data/costly-loop/0.4.25/multiple_costly_operations_in_loop.sol#5-9) has costly operations inside a loop:
 	- state_variable_base ++ (tests/e2e/detectors/test_data/costly-loop/0.4.25/multiple_costly_operations_in_loop.sol#7)
 
-CostlyOperationsInLoop.bad3_internal() (tests/e2e/detectors/test_data/costly-loop/0.4.25/multiple_costly_operations_in_loop.sol#41-43) has costly operations inside a loop:
-	- state_variable ++ (tests/e2e/detectors/test_data/costly-loop/0.4.25/multiple_costly_operations_in_loop.sol#42)
-
 CostlyOperationsInLoop.bad2() (tests/e2e/detectors/test_data/costly-loop/0.4.25/multiple_costly_operations_in_loop.sol#26-33) has costly operations inside a loop:
 	- state_variable ++ (tests/e2e/detectors/test_data/costly-loop/0.4.25/multiple_costly_operations_in_loop.sol#31)
 
+CostlyOperationsInLoop.bad3_internal() (tests/e2e/detectors/test_data/costly-loop/0.4.25/multiple_costly_operations_in_loop.sol#41-43) has costly operations inside a loop:
+	- state_variable ++ (tests/e2e/detectors/test_data/costly-loop/0.4.25/multiple_costly_operations_in_loop.sol#42)
+	Calls stack containing the loop:
+		CostlyOperationsInLoop.bad3()
+
 CostlyOperationsInLoop.bad() (tests/e2e/detectors/test_data/costly-loop/0.4.25/multiple_costly_operations_in_loop.sol#20-24) has costly operations inside a loop:
 	- state_variable ++ (tests/e2e/detectors/test_data/costly-loop/0.4.25/multiple_costly_operations_in_loop.sol#22)
 

Diff for: tests/e2e/detectors/snapshots/detectors__detector_CostlyOperationsInLoop_0_5_16_multiple_costly_operations_in_loop_sol__0.txt

@@ -6,6 +6,8 @@ CostlyOperationsInLoop.bad2() (tests/e2e/detectors/test_data/costly-loop/0.5.16/
 
 CostlyOperationsInLoop.bad3_internal() (tests/e2e/detectors/test_data/costly-loop/0.5.16/multiple_costly_operations_in_loop.sol#41-43) has costly operations inside a loop:
 	- state_variable ++ (tests/e2e/detectors/test_data/costly-loop/0.5.16/multiple_costly_operations_in_loop.sol#42)
+	Calls stack containing the loop:
+		CostlyOperationsInLoop.bad3()
 
 CostlyOperationsInLoop.bad() (tests/e2e/detectors/test_data/costly-loop/0.5.16/multiple_costly_operations_in_loop.sol#20-24) has costly operations inside a loop:
 	- state_variable ++ (tests/e2e/detectors/test_data/costly-loop/0.5.16/multiple_costly_operations_in_loop.sol#22)

Diff for: tests/e2e/detectors/snapshots/detectors__detector_CostlyOperationsInLoop_0_6_11_multiple_costly_operations_in_loop_sol__0.txt

@@ -4,9 +4,11 @@ CostlyOperationsInLoop.bad() (tests/e2e/detectors/test_data/costly-loop/0.6.11/m
 CostlyOperationsInLoop.bad2() (tests/e2e/detectors/test_data/costly-loop/0.6.11/multiple_costly_operations_in_loop.sol#26-33) has costly operations inside a loop:
 	- state_variable ++ (tests/e2e/detectors/test_data/costly-loop/0.6.11/multiple_costly_operations_in_loop.sol#31)
 
-CostlyOperationsInLoop.bad3_internal() (tests/e2e/detectors/test_data/costly-loop/0.6.11/multiple_costly_operations_in_loop.sol#41-43) has costly operations inside a loop:
-	- state_variable ++ (tests/e2e/detectors/test_data/costly-loop/0.6.11/multiple_costly_operations_in_loop.sol#42)
-
 CostlyOperationsInLoopBase.bad_base() (tests/e2e/detectors/test_data/costly-loop/0.6.11/multiple_costly_operations_in_loop.sol#5-9) has costly operations inside a loop:
 	- state_variable_base ++ (tests/e2e/detectors/test_data/costly-loop/0.6.11/multiple_costly_operations_in_loop.sol#7)
 
+CostlyOperationsInLoop.bad3_internal() (tests/e2e/detectors/test_data/costly-loop/0.6.11/multiple_costly_operations_in_loop.sol#41-43) has costly operations inside a loop:
+	- state_variable ++ (tests/e2e/detectors/test_data/costly-loop/0.6.11/multiple_costly_operations_in_loop.sol#42)
+	Calls stack containing the loop:
+		CostlyOperationsInLoop.bad3()
+

Diff for: tests/e2e/detectors/snapshots/detectors__detector_CostlyOperationsInLoop_0_7_6_multiple_costly_operations_in_loop_sol__0.txt

@@ -1,12 +1,14 @@
-CostlyOperationsInLoop.bad3_internal() (tests/e2e/detectors/test_data/costly-loop/0.7.6/multiple_costly_operations_in_loop.sol#41-43) has costly operations inside a loop:
-	- state_variable ++ (tests/e2e/detectors/test_data/costly-loop/0.7.6/multiple_costly_operations_in_loop.sol#42)
-
 CostlyOperationsInLoopBase.bad_base() (tests/e2e/detectors/test_data/costly-loop/0.7.6/multiple_costly_operations_in_loop.sol#5-9) has costly operations inside a loop:
 	- state_variable_base ++ (tests/e2e/detectors/test_data/costly-loop/0.7.6/multiple_costly_operations_in_loop.sol#7)
 
 CostlyOperationsInLoop.bad2() (tests/e2e/detectors/test_data/costly-loop/0.7.6/multiple_costly_operations_in_loop.sol#26-33) has costly operations inside a loop:
 	- state_variable ++ (tests/e2e/detectors/test_data/costly-loop/0.7.6/multiple_costly_operations_in_loop.sol#31)
 
+CostlyOperationsInLoop.bad3_internal() (tests/e2e/detectors/test_data/costly-loop/0.7.6/multiple_costly_operations_in_loop.sol#41-43) has costly operations inside a loop:
+	- state_variable ++ (tests/e2e/detectors/test_data/costly-loop/0.7.6/multiple_costly_operations_in_loop.sol#42)
+	Calls stack containing the loop:
+		CostlyOperationsInLoop.bad3()
+
 CostlyOperationsInLoop.bad() (tests/e2e/detectors/test_data/costly-loop/0.7.6/multiple_costly_operations_in_loop.sol#20-24) has costly operations inside a loop:
 	- state_variable ++ (tests/e2e/detectors/test_data/costly-loop/0.7.6/multiple_costly_operations_in_loop.sol#22)
 

Diff for: tests/e2e/detectors/snapshots/detectors__detector_DelegatecallInLoop_0_4_25_delegatecall_loop_sol__0.txt

@@ -1,6 +1,8 @@
+C.bad2_internal(address[]) (tests/e2e/detectors/test_data/delegatecall-loop/0.4.25/delegatecall_loop.sol#19-23) has delegatecall inside a loop in a payable function: address(this).delegatecall(abi.encodeWithSignature(addBalance(address),receivers[i])) (tests/e2e/detectors/test_data/delegatecall-loop/0.4.25/delegatecall_loop.sol#21)
+	Calls stack containing the loop:
+		C.bad2(address[])
+
 C.bad(address[]) (tests/e2e/detectors/test_data/delegatecall-loop/0.4.25/delegatecall_loop.sol#5-9) has delegatecall inside a loop in a payable function: address(this).delegatecall(abi.encodeWithSignature(addBalance(address),receivers[i])) (tests/e2e/detectors/test_data/delegatecall-loop/0.4.25/delegatecall_loop.sol#7)
 
 C.bad3(address[]) (tests/e2e/detectors/test_data/delegatecall-loop/0.4.25/delegatecall_loop.sol#25-31) has delegatecall inside a loop in a payable function: address(this).delegatecall(abi.encodeWithSignature(addBalance(address),receivers[i])) (tests/e2e/detectors/test_data/delegatecall-loop/0.4.25/delegatecall_loop.sol#28)
 
-C.bad2_internal(address[]) (tests/e2e/detectors/test_data/delegatecall-loop/0.4.25/delegatecall_loop.sol#19-23) has delegatecall inside a loop in a payable function: address(this).delegatecall(abi.encodeWithSignature(addBalance(address),receivers[i])) (tests/e2e/detectors/test_data/delegatecall-loop/0.4.25/delegatecall_loop.sol#21)
-