diff --git a/slither/detectors/operations/unchecked_low_level_return_values.py b/slither/detectors/operations/unchecked_low_level_return_values.py index 0537ebbf2..c1fb1a868 100644 --- a/slither/detectors/operations/unchecked_low_level_return_values.py +++ b/slither/detectors/operations/unchecked_low_level_return_values.py @@ -1,15 +1,24 @@ """ Module detecting unused return values from low level """ -from slither.detectors.abstract_detector import DetectorClassification -from slither.detectors.operations.unused_return_values import UnusedReturnValues +from typing import List + +from slither.core.cfg.node import Node from slither.slithir.operations import LowLevelCall -from slither.slithir.operations.operation import Operation + +from slither.core.declarations.function_contract import FunctionContract +from slither.core.variables.state_variable import StateVariable +from slither.detectors.abstract_detector import ( + AbstractDetector, + DetectorClassification, + DETECTOR_INFO, +) +from slither.utils.output import Output -class UncheckedLowLevel(UnusedReturnValues): +class UncheckedLowLevel(AbstractDetector): """ - If the return value of a send is not checked, it might lead to losing ether + If the return value of a low-level call is not checked, it might lead to losing ether """ ARGUMENT = "unchecked-lowlevel" @@ -38,5 +47,44 @@ If the low level is used to prevent blocking operations, consider logging failed WIKI_RECOMMENDATION = "Ensure that the return value of a low-level call is checked or logged." - def _is_instance(self, ir: Operation) -> bool: # pylint: disable=no-self-use - return isinstance(ir, LowLevelCall) + @staticmethod + def detect_unused_return_values(f: FunctionContract) -> List[Node]: + """ + Return the nodes where the return value of a call is unused + Args: + f (Function) + Returns: + list(Node) + """ + values_returned = [] + nodes_origin = {} + for n in f.nodes: + for ir in n.irs: + if isinstance(ir, LowLevelCall): + # if a return value is stored in a state variable, it's ok + if ir.lvalue and not isinstance(ir.lvalue, StateVariable): + values_returned.append(ir.lvalue) + nodes_origin[ir.lvalue] = ir + + for read in ir.read: + if read in values_returned: + values_returned.remove(read) + + return [nodes_origin[value].node for value in values_returned] + + def _detect(self) -> List[Output]: + """Detect low level calls where the success value is not checked""" + results = [] + for c in self.compilation_unit.contracts_derived: + for f in c.functions_and_modifiers: + unused_return = UncheckedLowLevel.detect_unused_return_values(f) + if unused_return: + + for node in unused_return: + info: DETECTOR_INFO = [f, " ignores return value by ", node, "\n"] + + res = self.generate_result(info) + + results.append(res) + + return results diff --git a/slither/detectors/operations/unused_return_values.py b/slither/detectors/operations/unused_return_values.py index 80be98b45..783a44807 100644 --- a/slither/detectors/operations/unused_return_values.py +++ b/slither/detectors/operations/unused_return_values.py @@ -101,10 +101,8 @@ contract MyConc{ def _detect(self) -> List[Output]: """Detect high level calls which return a value that are never used""" results = [] - for c in self.compilation_unit.contracts: - for f in c.functions + c.modifiers: - if f.contract_declarer != c: - continue + for c in self.compilation_unit.contracts_derived: + for f in c.functions_and_modifiers: unused_return = self.detect_unused_return_values(f) if unused_return: diff --git a/tests/e2e/detectors/test_data/unchecked-lowlevel/0.7.6/unchecked_lowlevel.sol b/tests/e2e/detectors/test_data/unchecked-lowlevel/0.7.6/unchecked_lowlevel.sol index 96712b077..cd6f78e98 100644 --- a/tests/e2e/detectors/test_data/unchecked-lowlevel/0.7.6/unchecked_lowlevel.sol +++ b/tests/e2e/detectors/test_data/unchecked-lowlevel/0.7.6/unchecked_lowlevel.sol @@ -4,8 +4,14 @@ contract MyConc{ } function good(address payable dst) external payable{ - (bool ret, bytes memory _) = dst.call{value:msg.value}(""); + (bool ret, ) = dst.call{value:msg.value}(""); require(ret); } + function good2(address payable dst) external payable{ + (bool ret, ) = dst.call{value:msg.value}(""); + if (!ret) { + revert(); + } + } } diff --git a/tests/e2e/detectors/test_data/unchecked-lowlevel/0.7.6/unchecked_lowlevel.sol-0.7.6.zip b/tests/e2e/detectors/test_data/unchecked-lowlevel/0.7.6/unchecked_lowlevel.sol-0.7.6.zip index 530d247c4..bccb47182 100644 Binary files a/tests/e2e/detectors/test_data/unchecked-lowlevel/0.7.6/unchecked_lowlevel.sol-0.7.6.zip and b/tests/e2e/detectors/test_data/unchecked-lowlevel/0.7.6/unchecked_lowlevel.sol-0.7.6.zip differ