Skip to content
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

Add detector delegatecall inside a loop #992

Merged
merged 3 commits into from
Dec 8, 2021
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
1 change: 1 addition & 0 deletions slither/detectors/all_detectors.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
from .operations.missing_zero_address_validation import MissingZeroAddressValidation
from .functions.dead_code import DeadCode
from .statements.write_after_write import WriteAfterWrite
from .statements.delegatecall_in_loop import DelegatecallInLoop

#
#
97 changes: 97 additions & 0 deletions slither/detectors/statements/delegatecall_in_loop.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
from typing import List
from slither.core.cfg.node import NodeType, Node
from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
from slither.slithir.operations import LowLevelCall, InternalCall
from slither.core.declarations import Contract
from slither.utils.output import Output


def detect_delegatecall_in_loop(contract: Contract) -> List[Node]:
results: List[Node] = []
for f in contract.functions_entry_points:
if f.is_implemented and f.payable:
delegatecall_in_loop(f.entry_point, 0, [], results)
return results


def delegatecall_in_loop(
node: Node, in_loop_counter: int, visited: List[Node], results: List[Node]
) -> None:
if node in visited:
return
# shared visited
visited.append(node)

if node.type == NodeType.STARTLOOP:
in_loop_counter += 1
elif node.type == NodeType.ENDLOOP:
in_loop_counter -= 1

for ir in node.all_slithir_operations():
if (
in_loop_counter > 0
and isinstance(ir, (LowLevelCall))
and ir.function_name == "delegatecall"
):
results.append(ir.node)
if isinstance(ir, (InternalCall)):
delegatecall_in_loop(ir.function.entry_point, in_loop_counter, visited, results)

for son in node.sons:
delegatecall_in_loop(son, in_loop_counter, visited, results)


class DelegatecallInLoop(AbstractDetector):
"""
Detect the use of delegatecall inside a loop in a payable function
"""

ARGUMENT = "delegatecall-loop"
HELP = "Payable functions using `delegatecall` inside a loop"
IMPACT = DetectorClassification.HIGH
CONFIDENCE = DetectorClassification.MEDIUM

WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation/#payable-functions-using-delegatecall-inside-a-loop"

WIKI_TITLE = "Payable functions using `delegatecall` inside a loop"
WIKI_DESCRIPTION = "Detect the use of `delegatecall` inside a loop in a payable function."

# region wiki_exploit_scenario
WIKI_EXPLOIT_SCENARIO = """
```solidity
contract DelegatecallInLoop{

mapping (address => uint256) balances;

function bad(address[] memory receivers) public payable {
for (uint256 i = 0; i < receivers.length; i++) {
address(this).delegatecall(abi.encodeWithSignature("addBalance(address)", receivers[i]));
}
}

function addBalance(address a) public payable {
balances[a] += msg.value;
}

}
```
When calling `bad` the same `msg.value` amount will be accredited multiple times."""
# endregion wiki_exploit_scenario

WIKI_RECOMMENDATION = """
Carefully check that the function called by `delegatecall` is not payable/doesn't use `msg.value`.
"""

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:
func = node.function

info = [func, " has delegatecall inside a loop in a payable function: ", node, "\n"]
res = self.generate_result(info)
results.append(res)

return results
33 changes: 33 additions & 0 deletions tests/detectors/delegatecall-loop/0.4.25/delegatecall_loop.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
contract C{

mapping (address => uint256) balances;

function bad(address[] memory receivers) public payable {
for (uint256 i = 0; i < receivers.length; i++) {
address(this).delegatecall(abi.encodeWithSignature("addBalance(address)", receivers[i]));
}
}

function addBalance(address a) public payable {
balances[a] += msg.value;
}

function bad2(address[] memory receivers) public payable {
bad2_internal(receivers);
}

function bad2_internal(address[] memory receivers) internal {
for (uint256 i = 0; i < receivers.length; i++) {
address(this).delegatecall(abi.encodeWithSignature("addBalance(address)", receivers[i]));
}
}

function bad3(address[] memory receivers) public payable {
for (uint256 i = 0; i < receivers.length; i++) {
for (uint256 j = 0; j < receivers.length; j++) {
address(this).delegatecall(abi.encodeWithSignature("addBalance(address)", receivers[i]));
}
}
}

}
Loading