Merge pull request #2029 from crytic/fix/fp/unchecked-lowlevel

fix regression that caused retdata to be flagged

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 typing import List
-from slither.detectors.operations.unused_return_values import UnusedReturnValues
+
+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
+    @staticmethod
-        return isinstance(ir, LowLevelCall)
+    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

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 c in self.compilation_unit.contracts_derived:
-            for f in c.functions + c.modifiers:
+            for f in c.functions_and_modifiers:
-                if f.contract_declarer != c:
-                    continue
                 unused_return = self.detect_unused_return_values(f)
                 if unused_return:
 

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();
+        }
+    }
 }