diff --git a/slither/detectors/operations/unused_return_values.py b/slither/detectors/operations/unused_return_values.py index 93dda274aa..80be98b45d 100644 --- a/slither/detectors/operations/unused_return_values.py +++ b/slither/detectors/operations/unused_return_values.py @@ -3,7 +3,7 @@ """ from typing import List -from slither.core.cfg.node import Node +from slither.core.cfg.node import Node, NodeType from slither.core.declarations import Function from slither.core.declarations.function_contract import FunctionContract from slither.core.variables.state_variable import StateVariable @@ -12,8 +12,8 @@ DetectorClassification, DETECTOR_INFO, ) -from slither.slithir.operations import HighLevelCall -from slither.slithir.operations.operation import Operation +from slither.slithir.operations import HighLevelCall, Assignment, Unpack, Operation +from slither.slithir.variables import TupleVariable from slither.utils.output import Output @@ -50,13 +50,18 @@ class UnusedReturnValues(AbstractDetector): WIKI_RECOMMENDATION = "Ensure that all the return values of the function calls are used." def _is_instance(self, ir: Operation) -> bool: # pylint: disable=no-self-use - return isinstance(ir, HighLevelCall) and ( - ( - isinstance(ir.function, Function) - and ir.function.solidity_signature - not in ["transfer(address,uint256)", "transferFrom(address,address,uint256)"] + return ( + isinstance(ir, HighLevelCall) + and ( + ( + isinstance(ir.function, Function) + and ir.function.solidity_signature + not in ["transfer(address,uint256)", "transferFrom(address,address,uint256)"] + ) + or not isinstance(ir.function, Function) ) - or not isinstance(ir.function, Function) + or ir.node.type == NodeType.TRY + and isinstance(ir, (Assignment, Unpack)) ) def detect_unused_return_values( @@ -71,18 +76,27 @@ def detect_unused_return_values( """ values_returned = [] nodes_origin = {} + # pylint: disable=too-many-nested-blocks for n in f.nodes: for ir in n.irs: if self._is_instance(ir): # 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) + values_returned.append((ir.lvalue, None)) nodes_origin[ir.lvalue] = ir + if isinstance(ir.lvalue, TupleVariable): + # we iterate the number of elements the tuple has + # and add a (variable, index) in values_returned for each of them + for index in range(len(ir.lvalue.type)): + values_returned.append((ir.lvalue, index)) for read in ir.read: - if read in values_returned: - values_returned.remove(read) - - return [nodes_origin[value].node for value in values_returned] + remove = (read, ir.index) if isinstance(ir, Unpack) else (read, None) + if remove in values_returned: + # this is needed to remove the tuple variable when the first time one of its element is used + if remove[1] is not None and (remove[0], None) in values_returned: + values_returned.remove((remove[0], None)) + values_returned.remove(remove) + return [nodes_origin[value].node for (value, _) in values_returned] def _detect(self) -> List[Output]: """Detect high level calls which return a value that are never used""" diff --git a/tests/e2e/detectors/snapshots/detectors__detector_UnusedReturnValues_0_4_25_unused_return_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_UnusedReturnValues_0_4_25_unused_return_sol__0.txt index ec28fa9e54..2d70ddd1cf 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_UnusedReturnValues_0_4_25_unused_return_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_UnusedReturnValues_0_4_25_unused_return_sol__0.txt @@ -1,4 +1,8 @@ -User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.4.25/unused_return.sol#17-29) ignores return value by t.f() (tests/e2e/detectors/test_data/unused-return/0.4.25/unused_return.sol#18) +User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.4.25/unused_return.sol#18-37) ignores return value by t.g() (tests/e2e/detectors/test_data/unused-return/0.4.25/unused_return.sol#31) -User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.4.25/unused_return.sol#17-29) ignores return value by a.add(0) (tests/e2e/detectors/test_data/unused-return/0.4.25/unused_return.sol#22) +User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.4.25/unused_return.sol#18-37) ignores return value by t.f() (tests/e2e/detectors/test_data/unused-return/0.4.25/unused_return.sol#19) + +User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.4.25/unused_return.sol#18-37) ignores return value by a.add(0) (tests/e2e/detectors/test_data/unused-return/0.4.25/unused_return.sol#23) + +User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.4.25/unused_return.sol#18-37) ignores return value by (e) = t.g() (tests/e2e/detectors/test_data/unused-return/0.4.25/unused_return.sol#36) diff --git a/tests/e2e/detectors/snapshots/detectors__detector_UnusedReturnValues_0_5_16_unused_return_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_UnusedReturnValues_0_5_16_unused_return_sol__0.txt index 0cf04d2833..5a651712e5 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_UnusedReturnValues_0_5_16_unused_return_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_UnusedReturnValues_0_5_16_unused_return_sol__0.txt @@ -1,4 +1,8 @@ -User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.5.16/unused_return.sol#17-29) ignores return value by t.f() (tests/e2e/detectors/test_data/unused-return/0.5.16/unused_return.sol#18) +User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.5.16/unused_return.sol#18-37) ignores return value by (e) = t.g() (tests/e2e/detectors/test_data/unused-return/0.5.16/unused_return.sol#36) -User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.5.16/unused_return.sol#17-29) ignores return value by a.add(0) (tests/e2e/detectors/test_data/unused-return/0.5.16/unused_return.sol#22) +User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.5.16/unused_return.sol#18-37) ignores return value by a.add(0) (tests/e2e/detectors/test_data/unused-return/0.5.16/unused_return.sol#23) + +User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.5.16/unused_return.sol#18-37) ignores return value by t.g() (tests/e2e/detectors/test_data/unused-return/0.5.16/unused_return.sol#31) + +User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.5.16/unused_return.sol#18-37) ignores return value by t.f() (tests/e2e/detectors/test_data/unused-return/0.5.16/unused_return.sol#19) diff --git a/tests/e2e/detectors/snapshots/detectors__detector_UnusedReturnValues_0_6_11_unused_return_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_UnusedReturnValues_0_6_11_unused_return_sol__0.txt index be0a8c6875..5f1d751b7c 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_UnusedReturnValues_0_6_11_unused_return_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_UnusedReturnValues_0_6_11_unused_return_sol__0.txt @@ -1,4 +1,8 @@ -User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.6.11/unused_return.sol#17-29) ignores return value by a.add(0) (tests/e2e/detectors/test_data/unused-return/0.6.11/unused_return.sol#22) +User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.6.11/unused_return.sol#18-37) ignores return value by t.f() (tests/e2e/detectors/test_data/unused-return/0.6.11/unused_return.sol#19) -User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.6.11/unused_return.sol#17-29) ignores return value by t.f() (tests/e2e/detectors/test_data/unused-return/0.6.11/unused_return.sol#18) +User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.6.11/unused_return.sol#18-37) ignores return value by a.add(0) (tests/e2e/detectors/test_data/unused-return/0.6.11/unused_return.sol#23) + +User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.6.11/unused_return.sol#18-37) ignores return value by t.g() (tests/e2e/detectors/test_data/unused-return/0.6.11/unused_return.sol#31) + +User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.6.11/unused_return.sol#18-37) ignores return value by (e) = t.g() (tests/e2e/detectors/test_data/unused-return/0.6.11/unused_return.sol#36) diff --git a/tests/e2e/detectors/snapshots/detectors__detector_UnusedReturnValues_0_7_6_unused_return_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_UnusedReturnValues_0_7_6_unused_return_sol__0.txt index ec74a9458d..4780c79057 100644 --- a/tests/e2e/detectors/snapshots/detectors__detector_UnusedReturnValues_0_7_6_unused_return_sol__0.txt +++ b/tests/e2e/detectors/snapshots/detectors__detector_UnusedReturnValues_0_7_6_unused_return_sol__0.txt @@ -1,4 +1,8 @@ -User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.7.6/unused_return.sol#17-29) ignores return value by a.add(0) (tests/e2e/detectors/test_data/unused-return/0.7.6/unused_return.sol#22) +User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.7.6/unused_return.sol#18-37) ignores return value by t.g() (tests/e2e/detectors/test_data/unused-return/0.7.6/unused_return.sol#31) -User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.7.6/unused_return.sol#17-29) ignores return value by t.f() (tests/e2e/detectors/test_data/unused-return/0.7.6/unused_return.sol#18) +User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.7.6/unused_return.sol#18-37) ignores return value by a.add(0) (tests/e2e/detectors/test_data/unused-return/0.7.6/unused_return.sol#23) + +User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.7.6/unused_return.sol#18-37) ignores return value by t.f() (tests/e2e/detectors/test_data/unused-return/0.7.6/unused_return.sol#19) + +User.test(Target) (tests/e2e/detectors/test_data/unused-return/0.7.6/unused_return.sol#18-37) ignores return value by (e) = t.g() (tests/e2e/detectors/test_data/unused-return/0.7.6/unused_return.sol#36) diff --git a/tests/e2e/detectors/test_data/unused-return/0.4.25/unused_return.sol b/tests/e2e/detectors/test_data/unused-return/0.4.25/unused_return.sol index d1c1895987..ef22b63aed 100644 --- a/tests/e2e/detectors/test_data/unused-return/0.4.25/unused_return.sol +++ b/tests/e2e/detectors/test_data/unused-return/0.4.25/unused_return.sol @@ -8,6 +8,7 @@ library SafeMath{ contract Target{ function f() public returns(uint); + function g() public returns(uint, uint); } contract User{ @@ -26,5 +27,12 @@ contract User{ // As the value returned by the call is stored // (unused local variable should be another issue) uint b = a.add(1); + + t.g(); + + (uint c, uint d) = t.g(); + + // Detected as unused return + (uint e,) = t.g(); } } diff --git a/tests/e2e/detectors/test_data/unused-return/0.4.25/unused_return.sol-0.4.25.zip b/tests/e2e/detectors/test_data/unused-return/0.4.25/unused_return.sol-0.4.25.zip index bfe469a89a..8f64587ae6 100644 Binary files a/tests/e2e/detectors/test_data/unused-return/0.4.25/unused_return.sol-0.4.25.zip and b/tests/e2e/detectors/test_data/unused-return/0.4.25/unused_return.sol-0.4.25.zip differ diff --git a/tests/e2e/detectors/test_data/unused-return/0.5.16/unused_return.sol b/tests/e2e/detectors/test_data/unused-return/0.5.16/unused_return.sol index d1c1895987..ef22b63aed 100644 --- a/tests/e2e/detectors/test_data/unused-return/0.5.16/unused_return.sol +++ b/tests/e2e/detectors/test_data/unused-return/0.5.16/unused_return.sol @@ -8,6 +8,7 @@ library SafeMath{ contract Target{ function f() public returns(uint); + function g() public returns(uint, uint); } contract User{ @@ -26,5 +27,12 @@ contract User{ // As the value returned by the call is stored // (unused local variable should be another issue) uint b = a.add(1); + + t.g(); + + (uint c, uint d) = t.g(); + + // Detected as unused return + (uint e,) = t.g(); } } diff --git a/tests/e2e/detectors/test_data/unused-return/0.5.16/unused_return.sol-0.5.16.zip b/tests/e2e/detectors/test_data/unused-return/0.5.16/unused_return.sol-0.5.16.zip index 90d7768695..13d0d7a323 100644 Binary files a/tests/e2e/detectors/test_data/unused-return/0.5.16/unused_return.sol-0.5.16.zip and b/tests/e2e/detectors/test_data/unused-return/0.5.16/unused_return.sol-0.5.16.zip differ diff --git a/tests/e2e/detectors/test_data/unused-return/0.6.11/unused_return.sol b/tests/e2e/detectors/test_data/unused-return/0.6.11/unused_return.sol index 08d0eb3a59..279ac627e7 100644 --- a/tests/e2e/detectors/test_data/unused-return/0.6.11/unused_return.sol +++ b/tests/e2e/detectors/test_data/unused-return/0.6.11/unused_return.sol @@ -8,6 +8,7 @@ library SafeMath{ abstract contract Target{ function f() public virtual returns(uint); + function g() public virtual returns(uint, uint); } contract User{ @@ -26,5 +27,12 @@ contract User{ // As the value returned by the call is stored // (unused local variable should be another issue) uint b = a.add(1); + + t.g(); + + (uint c, uint d) = t.g(); + + // Detected as unused return + (uint e,) = t.g(); } } diff --git a/tests/e2e/detectors/test_data/unused-return/0.6.11/unused_return.sol-0.6.11.zip b/tests/e2e/detectors/test_data/unused-return/0.6.11/unused_return.sol-0.6.11.zip index 5f04b6a004..30ae730404 100644 Binary files a/tests/e2e/detectors/test_data/unused-return/0.6.11/unused_return.sol-0.6.11.zip and b/tests/e2e/detectors/test_data/unused-return/0.6.11/unused_return.sol-0.6.11.zip differ diff --git a/tests/e2e/detectors/test_data/unused-return/0.7.6/unused_return.sol b/tests/e2e/detectors/test_data/unused-return/0.7.6/unused_return.sol index 08d0eb3a59..279ac627e7 100644 --- a/tests/e2e/detectors/test_data/unused-return/0.7.6/unused_return.sol +++ b/tests/e2e/detectors/test_data/unused-return/0.7.6/unused_return.sol @@ -8,6 +8,7 @@ library SafeMath{ abstract contract Target{ function f() public virtual returns(uint); + function g() public virtual returns(uint, uint); } contract User{ @@ -26,5 +27,12 @@ contract User{ // As the value returned by the call is stored // (unused local variable should be another issue) uint b = a.add(1); + + t.g(); + + (uint c, uint d) = t.g(); + + // Detected as unused return + (uint e,) = t.g(); } } diff --git a/tests/e2e/detectors/test_data/unused-return/0.7.6/unused_return.sol-0.7.6.zip b/tests/e2e/detectors/test_data/unused-return/0.7.6/unused_return.sol-0.7.6.zip index ac668ccc92..08ecd82e6a 100644 Binary files a/tests/e2e/detectors/test_data/unused-return/0.7.6/unused_return.sol-0.7.6.zip and b/tests/e2e/detectors/test_data/unused-return/0.7.6/unused_return.sol-0.7.6.zip differ