Skip to content

Add calls stack information to detectors #2696

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 29 additions & 16 deletions slither/detectors/statements/calls_in_loop.py
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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)

Expand Down
36 changes: 26 additions & 10 deletions slither/detectors/statements/costly_operations_in_loop.py
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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:
Expand All @@ -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):
Expand Down Expand Up @@ -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)

Expand Down
36 changes: 26 additions & 10 deletions slither/detectors/statements/delegatecall_in_loop.py
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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:
Expand All @@ -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):
Expand Down Expand Up @@ -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 = [
Expand All @@ -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)

Expand Down
36 changes: 26 additions & 10 deletions slither/detectors/statements/msg_value_in_loop.py
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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:
Expand All @@ -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:
Expand All @@ -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):
Expand Down Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
@@ -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)

Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Original file line number Diff line number Diff line change
@@ -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)

Original file line number Diff line number Diff line change
@@ -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)

Loading