diff --git a/slither/detectors/assembly/incorrect_return.py b/slither/detectors/assembly/incorrect_return.py index 48ecab178..9052979ac 100644 --- a/slither/detectors/assembly/incorrect_return.py +++ b/slither/detectors/assembly/incorrect_return.py @@ -21,10 +21,8 @@ def _assembly_node(function: Function) -> Optional[SolidityCall]: """ - for ir in function.all_slithir_operations(): - if isinstance(ir, SolidityCall) and ir.function == SolidityFunction( - "return(uint256,uint256)" - ): + for ir in function.all_solidity_calls(): + if ir.function == SolidityFunction("return(uint256,uint256)"): return ir return None @@ -71,24 +69,23 @@ The function will return 6 bytes starting from offset 5, instead of returning a for c in self.contracts: for f in c.functions_and_modifiers_declared: - for node in f.nodes: - if node.sons: - for ir in node.internal_calls: - function_called = ir.function - if isinstance(function_called, Function): - found = _assembly_node(function_called) - if found: - - info: DETECTOR_INFO = [ - f, - " calls ", - function_called, - " which halt the execution ", - found.node, - "\n", - ] - json = self.generate_result(info) - - results.append(json) + for ir in f.internal_calls: + if ir.node.sons: + function_called = ir.function + if isinstance(function_called, Function): + found = _assembly_node(function_called) + if found: + + info: DETECTOR_INFO = [ + f, + " calls ", + function_called, + " which halt the execution ", + found.node, + "\n", + ] + json = self.generate_result(info) + + results.append(json) return results diff --git a/slither/detectors/assembly/return_instead_of_leave.py b/slither/detectors/assembly/return_instead_of_leave.py index a1ad9c87e..603705974 100644 --- a/slither/detectors/assembly/return_instead_of_leave.py +++ b/slither/detectors/assembly/return_instead_of_leave.py @@ -6,7 +6,6 @@ from slither.detectors.abstract_detector import ( DetectorClassification, DETECTOR_INFO, ) -from slither.slithir.operations import SolidityCall from slither.utils.output import Output @@ -42,15 +41,12 @@ The function will halt the execution, instead of returning a two uint.""" def _check_function(self, f: Function) -> List[Output]: results: List[Output] = [] - for node in f.nodes: - for ir in node.irs: - if isinstance(ir, SolidityCall) and ir.function == SolidityFunction( - "return(uint256,uint256)" - ): - info: DETECTOR_INFO = [f, " contains an incorrect call to return: ", node, "\n"] - json = self.generate_result(info) + for ir in f.solidity_calls: + if ir.function == SolidityFunction("return(uint256,uint256)"): + info: DETECTOR_INFO = [f, " contains an incorrect call to return: ", ir.node, "\n"] + json = self.generate_result(info) - results.append(json) + results.append(json) return results def _detect(self) -> List[Output]: diff --git a/slither/detectors/compiler_bugs/array_by_reference.py b/slither/detectors/compiler_bugs/array_by_reference.py index 47e2af581..e4dde4360 100644 --- a/slither/detectors/compiler_bugs/array_by_reference.py +++ b/slither/detectors/compiler_bugs/array_by_reference.py @@ -13,8 +13,6 @@ from slither.detectors.abstract_detector import ( from slither.core.solidity_types.array_type import ArrayType from slither.core.variables.state_variable import StateVariable from slither.core.variables.local_variable import LocalVariable -from slither.slithir.operations.high_level_call import HighLevelCall -from slither.slithir.operations.internal_call import InternalCall from slither.core.cfg.node import Node from slither.core.declarations.contract import Contract from slither.core.declarations.function_contract import FunctionContract @@ -117,37 +115,26 @@ As a result, Bob's usage of the contract is incorrect.""" # pylint: disable=too-many-nested-blocks for contract in contracts: for function in contract.functions_and_modifiers_declared: - for node in function.nodes: + for ir in [ir for _, ir in function.high_level_calls] + function.internal_calls: - # If this node has no expression, skip it. - if not node.expression: + # Verify this references a function in our array modifying functions collection. + if ir.function not in array_modifying_funcs: continue - for ir in node.irs: - # Verify this is a high level call. - if not isinstance(ir, (HighLevelCall, InternalCall)): + # Verify one of these parameters is an array in storage. + for (param, arg) in zip(ir.function.parameters, ir.arguments): + # Verify this argument is a variable that is an array type. + if not isinstance(arg, (StateVariable, LocalVariable)): continue - - # Verify this references a function in our array modifying functions collection. - if ir.function not in array_modifying_funcs: + if not isinstance(arg.type, ArrayType): continue - # Verify one of these parameters is an array in storage. - for (param, arg) in zip(ir.function.parameters, ir.arguments): - # Verify this argument is a variable that is an array type. - if not isinstance(arg, (StateVariable, LocalVariable)): - continue - if not isinstance(arg.type, ArrayType): - continue - - # If it is a state variable OR a local variable referencing storage, we add it to the list. - if ( - isinstance(arg, StateVariable) - or (isinstance(arg, LocalVariable) and arg.location == "storage") - ) and ( - isinstance(param.type, ArrayType) and param.location != "storage" - ): - results.append((node, arg, ir.function)) + # If it is a state variable OR a local variable referencing storage, we add it to the list. + if ( + isinstance(arg, StateVariable) + or (isinstance(arg, LocalVariable) and arg.location == "storage") + ) and (isinstance(param.type, ArrayType) and param.location != "storage"): + results.append((ir.node, arg, ir.function)) return results def _detect(self) -> List[Output]: diff --git a/slither/detectors/erc/erc20/arbitrary_send_erc20.py b/slither/detectors/erc/erc20/arbitrary_send_erc20.py index 046b53ccb..4dc1f8db5 100644 --- a/slither/detectors/erc/erc20/arbitrary_send_erc20.py +++ b/slither/detectors/erc/erc20/arbitrary_send_erc20.py @@ -3,7 +3,7 @@ from typing import List from slither.analyses.data_dependency.data_dependency import is_dependent from slither.core.cfg.node import Node from slither.core.compilation_unit import SlitherCompilationUnit -from slither.core.declarations import Contract, Function, SolidityVariableComposed +from slither.core.declarations import Contract, Function, SolidityVariableComposed, FunctionContract from slither.core.declarations.solidity_variables import SolidityVariable from slither.slithir.operations import HighLevelCall, LibraryCall @@ -44,51 +44,50 @@ class ArbitrarySendErc20: "permit(address,address,uint256,uint256,uint8,bytes32,bytes32)" in all_high_level_calls ): - ArbitrarySendErc20._arbitrary_from(f.nodes, self._permit_results) + ArbitrarySendErc20._arbitrary_from(f, self._permit_results) else: - ArbitrarySendErc20._arbitrary_from(f.nodes, self._no_permit_results) + ArbitrarySendErc20._arbitrary_from(f, self._no_permit_results) @staticmethod - def _arbitrary_from(nodes: List[Node], results: List[Node]) -> None: + def _arbitrary_from(function: FunctionContract, results: List[Node]) -> None: """Finds instances of (safe)transferFrom that do not use msg.sender or address(this) as from parameter.""" - for node in nodes: - for ir in node.irs: - if ( - isinstance(ir, HighLevelCall) - and isinstance(ir.function, Function) - and ir.function.solidity_signature == "transferFrom(address,address,uint256)" - and not ( - is_dependent( - ir.arguments[0], - SolidityVariableComposed("msg.sender"), - node, - ) - or is_dependent( - ir.arguments[0], - SolidityVariable("this"), - node, - ) + for _, ir in function.high_level_calls: + if ( + isinstance(ir, LibraryCall) + and ir.function.solidity_signature + == "safeTransferFrom(address,address,address,uint256)" + and not ( + is_dependent( + ir.arguments[1], + SolidityVariableComposed("msg.sender"), + ir.node, ) - ): - results.append(ir.node) - elif ( - isinstance(ir, LibraryCall) - and ir.function.solidity_signature - == "safeTransferFrom(address,address,address,uint256)" - and not ( - is_dependent( - ir.arguments[1], - SolidityVariableComposed("msg.sender"), - node, - ) - or is_dependent( - ir.arguments[1], - SolidityVariable("this"), - node, - ) + or is_dependent( + ir.arguments[1], + SolidityVariable("this"), + ir.node, ) - ): - results.append(ir.node) + ) + ): + results.append(ir.node) + elif ( + isinstance(ir, HighLevelCall) + and isinstance(ir.function, Function) + and ir.function.solidity_signature == "transferFrom(address,address,uint256)" + and not ( + is_dependent( + ir.arguments[0], + SolidityVariableComposed("msg.sender"), + ir.node, + ) + or is_dependent( + ir.arguments[0], + SolidityVariable("this"), + ir.node, + ) + ) + ): + results.append(ir.node) def detect(self) -> None: """Detect transfers that use arbitrary `from` parameter.""" diff --git a/slither/detectors/functions/external_function.py b/slither/detectors/functions/external_function.py index 5858c2baf..c59fe4492 100644 --- a/slither/detectors/functions/external_function.py +++ b/slither/detectors/functions/external_function.py @@ -13,8 +13,7 @@ from slither.detectors.abstract_detector import ( make_solc_versions, ) from slither.formatters.functions.external_function import custom_format -from slither.slithir.operations import InternalCall, InternalDynamicCall -from slither.slithir.operations import SolidityCall +from slither.slithir.operations import InternalDynamicCall from slither.utils.output import Output @@ -55,11 +54,11 @@ class ExternalFunction(AbstractDetector): for func in contract.all_functions_called: if not isinstance(func, Function): continue - # Loop through all nodes in the function, add all calls to a list. - for node in func.nodes: - for ir in node.irs: - if isinstance(ir, (InternalCall, SolidityCall)): - result.append(ir.function) + + # Loop through all internal and solidity calls in the function, add them to a list. + for ir in func.internal_calls + func.solidity_calls: + result.append(ir.function) + return result @staticmethod diff --git a/slither/detectors/operations/encode_packed.py b/slither/detectors/operations/encode_packed.py index ea7b094df..b661ddcd7 100644 --- a/slither/detectors/operations/encode_packed.py +++ b/slither/detectors/operations/encode_packed.py @@ -3,14 +3,14 @@ Module detecting usage of more than one dynamic type in abi.encodePacked() argum """ from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification -from slither.core.declarations.solidity_variables import SolidityFunction -from slither.slithir.operations import SolidityCall +from slither.core.declarations import Contract, SolidityFunction +from slither.core.variables import Variable from slither.analyses.data_dependency.data_dependency import is_tainted from slither.core.solidity_types import ElementaryType from slither.core.solidity_types import ArrayType -def _is_dynamic_type(arg): +def _is_dynamic_type(arg: Variable): """ Args: arg (function argument) @@ -25,7 +25,7 @@ def _is_dynamic_type(arg): return False -def _detect_abi_encodePacked_collision(contract): +def _detect_abi_encodePacked_collision(contract: Contract): """ Args: contract (Contract) @@ -35,22 +35,19 @@ def _detect_abi_encodePacked_collision(contract): ret = [] # pylint: disable=too-many-nested-blocks for f in contract.functions_and_modifiers_declared: - for n in f.nodes: - for ir in n.irs: - if isinstance(ir, SolidityCall) and ir.function == SolidityFunction( - "abi.encodePacked()" - ): - dynamic_type_count = 0 - for arg in ir.arguments: - if is_tainted(arg, contract) and _is_dynamic_type(arg): - dynamic_type_count += 1 - elif dynamic_type_count > 1: - ret.append((f, n)) - dynamic_type_count = 0 - else: - dynamic_type_count = 0 - if dynamic_type_count > 1: - ret.append((f, n)) + for ir in f.solidity_calls: + if ir.function == SolidityFunction("abi.encodePacked()"): + dynamic_type_count = 0 + for arg in ir.arguments: + if is_tainted(arg, contract) and _is_dynamic_type(arg): + dynamic_type_count += 1 + elif dynamic_type_count > 1: + ret.append((f, ir.node)) + dynamic_type_count = 0 + else: + dynamic_type_count = 0 + if dynamic_type_count > 1: + ret.append((f, ir.node)) return ret diff --git a/slither/detectors/operations/low_level_calls.py b/slither/detectors/operations/low_level_calls.py index 463c74875..4925fc466 100644 --- a/slither/detectors/operations/low_level_calls.py +++ b/slither/detectors/operations/low_level_calls.py @@ -44,10 +44,9 @@ class LowLevelCalls(AbstractDetector): ) -> List[Tuple[FunctionContract, List[Node]]]: ret = [] for f in [f for f in contract.functions if contract == f.contract_declarer]: - nodes = f.nodes - assembly_nodes = [n for n in nodes if self._contains_low_level_calls(n)] - if assembly_nodes: - ret.append((f, assembly_nodes)) + low_level_nodes = [ir.node for ir in f.low_level_calls] + if low_level_nodes: + ret.append((f, low_level_nodes)) return ret def _detect(self) -> List[Output]: diff --git a/slither/detectors/statements/assert_state_change.py b/slither/detectors/statements/assert_state_change.py index 8a04f11e0..d70495365 100644 --- a/slither/detectors/statements/assert_state_change.py +++ b/slither/detectors/statements/assert_state_change.py @@ -30,22 +30,22 @@ def detect_assert_state_change( # Loop for each function and modifier. for function in contract.functions_declared + list(contract.modifiers_declared): - for node in function.nodes: + for ir_call in function.internal_calls: # Detect assert() calls - if any(ir.function.name == "assert(bool)" for ir in node.internal_calls) and ( + if ir_call.function.name == "assert(bool)" and ( # Detect direct changes to state - node.state_variables_written + ir_call.node.state_variables_written or # Detect changes to state via function calls any( ir - for ir in node.irs + for ir in ir_call.node.irs if isinstance(ir, InternalCall) and ir.function and ir.function.state_variables_written ) ): - results.append((function, node)) + results.append((function, ir_call.node)) # Return the resulting set of nodes return results diff --git a/slither/detectors/statements/controlled_delegatecall.py b/slither/detectors/statements/controlled_delegatecall.py index 32e59d6eb..bf78b3bf9 100644 --- a/slither/detectors/statements/controlled_delegatecall.py +++ b/slither/detectors/statements/controlled_delegatecall.py @@ -8,20 +8,18 @@ from slither.detectors.abstract_detector import ( DetectorClassification, DETECTOR_INFO, ) -from slither.slithir.operations import LowLevelCall from slither.utils.output import Output def controlled_delegatecall(function: FunctionContract) -> List[Node]: ret = [] - 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): - ret.append(node) + for ir in function.low_level_calls: + if ir.function_name in [ + "delegatecall", + "callcode", + ]: + if is_tainted(ir.destination, function.contract): + ret.append(ir.node) return ret diff --git a/slither/detectors/statements/return_bomb.py b/slither/detectors/statements/return_bomb.py index 8b6cd07a2..6d7052cf4 100644 --- a/slither/detectors/statements/return_bomb.py +++ b/slither/detectors/statements/return_bomb.py @@ -9,7 +9,7 @@ from slither.detectors.abstract_detector import ( DetectorClassification, DETECTOR_INFO, ) -from slither.slithir.operations import LowLevelCall, HighLevelCall +from slither.slithir.operations import HighLevelCall from slither.analyses.data_dependency.data_dependency import is_tainted from slither.utils.output import Output @@ -71,34 +71,31 @@ Callee unexpectedly makes the caller OOG. def get_nodes_for_function(self, function: Function, contract: Contract) -> List[Node]: nodes = [] - for node in function.nodes: - for ir in node.irs: - if isinstance(ir, (HighLevelCall, LowLevelCall)): - if not is_tainted(ir.destination, contract): # type:ignore - # Only interested if the target address is controlled/tainted - continue - - if isinstance(ir, HighLevelCall) and isinstance(ir.function, Function): - # in normal highlevel calls return bombs are _possible_ - # if the return type is dynamic and the caller tries to copy and decode large data - has_dyn = False - if ir.function.return_type: - has_dyn = any( - self.is_dynamic_type(ty) for ty in ir.function.return_type - ) - - if not has_dyn: - continue - - # If a gas budget was specified then the - # user may not know about the return bomb - if ir.call_gas is None: - # if a gas budget was NOT specified then the caller - # may already suspect the call may spend all gas? - continue - - nodes.append(node) - # TODO: check that there is some state change after the call + + for ir in [ir for _, ir in function.high_level_calls] + function.low_level_calls: + if not is_tainted(ir.destination, contract): # type:ignore + # Only interested if the target address is controlled/tainted + continue + + if isinstance(ir, HighLevelCall) and isinstance(ir.function, Function): + # in normal highlevel calls return bombs are _possible_ + # if the return type is dynamic and the caller tries to copy and decode large data + has_dyn = False + if ir.function.return_type: + has_dyn = any(self.is_dynamic_type(ty) for ty in ir.function.return_type) + + if not has_dyn: + continue + + # If a gas budget was specified then the + # user may not know about the return bomb + if ir.call_gas is None: + # if a gas budget was NOT specified then the caller + # may already suspect the call may spend all gas? + continue + + nodes.append(ir.node) + # TODO: check that there is some state change after the call return nodes diff --git a/slither/detectors/statements/unprotected_upgradeable.py b/slither/detectors/statements/unprotected_upgradeable.py index 681057749..aeb785da3 100644 --- a/slither/detectors/statements/unprotected_upgradeable.py +++ b/slither/detectors/statements/unprotected_upgradeable.py @@ -7,23 +7,28 @@ from slither.detectors.abstract_detector import ( DetectorClassification, DETECTOR_INFO, ) -from slither.slithir.operations import LowLevelCall, SolidityCall from slither.utils.output import Output def _can_be_destroyed(contract: Contract) -> List[Function]: targets = [] for f in contract.functions_entry_points: - for ir in f.all_slithir_operations(): - if ( - isinstance(ir, LowLevelCall) and ir.function_name in ["delegatecall", "codecall"] - ) or ( - isinstance(ir, SolidityCall) - and ir.function - in [SolidityFunction("suicide(address)"), SolidityFunction("selfdestruct(address)")] - ): + found = False + for ir in f.all_low_level_calls(): + if ir.function_name in ["delegatecall", "codecall"]: targets.append(f) + found = True break + + if not found: + for ir in f.all_solidity_calls(): + if ir.function in [ + SolidityFunction("suicide(address)"), + SolidityFunction("selfdestruct(address)"), + ]: + targets.append(f) + break + return targets diff --git a/slither/detectors/variables/var_read_using_this.py b/slither/detectors/variables/var_read_using_this.py index 537eecf8a..1e4787e36 100644 --- a/slither/detectors/variables/var_read_using_this.py +++ b/slither/detectors/variables/var_read_using_this.py @@ -7,7 +7,6 @@ from slither.detectors.abstract_detector import ( DetectorClassification, DETECTOR_INFO, ) -from slither.slithir.operations.high_level_call import HighLevelCall from slither.utils.output import Output @@ -54,13 +53,11 @@ contract C { @staticmethod def _detect_var_read_using_this(func: Function) -> List[Node]: results: List[Node] = [] - for node in func.nodes: - for ir in node.irs: - if isinstance(ir, HighLevelCall): - if ( - ir.destination == SolidityVariable("this") - and ir.is_static_call() - and ir.function.visibility == "public" - ): - results.append(node) + for _, ir in func.high_level_calls: + if ( + ir.destination == SolidityVariable("this") + and ir.is_static_call() + and ir.function.visibility == "public" + ): + results.append(ir.node) return sorted(results, key=lambda x: x.node_id)