Release 2 new detectors

slither/detectors/all_detectors.py

@@ -92,3 +92,5 @@ from .functions.cyclomatic_complexity import CyclomaticComplexity
 from .operations.cache_array_length import CacheArrayLength
 from .statements.incorrect_using_for import IncorrectUsingFor
 from .operations.encode_packed import EncodePackedCollision
+from .assembly.incorrect_return import IncorrectReturn
+from .assembly.return_instead_of_leave import ReturnInsteadOfLeave

slither/detectors/assembly/incorrect_return.py

@@ -0,0 +1,91 @@
+from typing import List, Optional
+
+from slither.core.declarations import SolidityFunction, Function
+from slither.detectors.abstract_detector import (
+    AbstractDetector,
+    DetectorClassification,
+    DETECTOR_INFO,
+)
+from slither.slithir.operations import SolidityCall
+from slither.utils.output import Output
+
+
+def _assembly_node(function: Function) -> Optional[SolidityCall]:
+    """
+    Check if there is a node that use return in assembly
+
+    Args:
+        function:
+
+    Returns:
+
+    """
+
+    for ir in function.all_slithir_operations():
+        if isinstance(ir, SolidityCall) and ir.function == SolidityFunction(
+            "return(uint256,uint256)"
+        ):
+            return ir
+    return None
+
+
+class IncorrectReturn(AbstractDetector):
+    """
+    Check for cases where a return(a,b) is used in an assembly function
+    """
+
+    ARGUMENT = "incorrect-return"
+    HELP = "If a `return` is incorrectly used in assembly mode."
+    IMPACT = DetectorClassification.HIGH
+    CONFIDENCE = DetectorClassification.MEDIUM
+
+    WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-assembly-return"
+
+    WIKI_TITLE = "Incorrect return in assembly"
+    WIKI_DESCRIPTION = "Detect if `return` in an assembly block halts unexpectedly the execution."
+    WIKI_EXPLOIT_SCENARIO = """
+```solidity
+contract C {
+    function f() internal returns (uint a, uint b) {
+        assembly {
+            return (5, 6)
+        }
+    }
+
+    function g() returns (bool){
+        f();
+        return true;
+    }
+}
+```
+The return statement in `f` will cause execution in `g` to halt.
+The function will return 6 bytes starting from offset 5, instead of returning a boolean."""
+
+    WIKI_RECOMMENDATION = "Use the `leave` statement."
+
+    # pylint: disable=too-many-nested-blocks
+    def _detect(self) -> List[Output]:
+        results: List[Output] = []
+        for c in self.contracts:
+            for f in c.functions_declared:
+
+                for node in f.nodes:
+                    if node.sons:
+                        for function_called in node.internal_calls:
+                            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

slither/detectors/assembly/return_instead_of_leave.py

@@ -0,0 +1,64 @@
+from typing import List
+
+from slither.core.declarations import SolidityFunction, Function
+from slither.detectors.abstract_detector import (
+    AbstractDetector,
+    DetectorClassification,
+    DETECTOR_INFO,
+)
+from slither.slithir.operations import SolidityCall
+from slither.utils.output import Output
+
+
+class ReturnInsteadOfLeave(AbstractDetector):
+    """
+    Check for cases where a return(a,b) is used in an assembly function that also returns two variables
+    """
+
+    ARGUMENT = "return-leave"
+    HELP = "If a `return` is used instead of a `leave`."
+    IMPACT = DetectorClassification.HIGH
+    CONFIDENCE = DetectorClassification.MEDIUM
+
+    WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-assembly-return"
+
+    WIKI_TITLE = "Return instead of leave in assembly"
+    WIKI_DESCRIPTION = "Detect if a `return` is used where a `leave` should be used."
+    WIKI_EXPLOIT_SCENARIO = """
+```solidity
+contract C {
+    function f() internal returns (uint a, uint b) {
+        assembly {
+            return (5, 6)
+        }
+    }
+
+}
+```
+The function will halt the execution, instead of returning a two uint."""
+
+    WIKI_RECOMMENDATION = "Use the `leave` statement."
+
+    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)
+
+                    results.append(json)
+        return results
+
+    def _detect(self) -> List[Output]:
+        results: List[Output] = []
+        for c in self.contracts:
+            for f in c.functions_declared:
+
+                if len(f.returns) == 2 and f.contains_assembly:
+                    results += self._check_function(f)
+
+        return results

tests/e2e/detectors/snapshots/detectors__detector_IncorrectReturn_0_8_10_incorrect_return_sol__0.txt

@@ -0,0 +1,4 @@
+C.bad1() (tests/e2e/detectors/test_data/incorrect-return/0.8.10/incorrect_return.sol#21-24) calls C.indirect() (tests/e2e/detectors/test_data/incorrect-return/0.8.10/incorrect_return.sol#17-19) which halt the execution return(uint256,uint256)(5,6) (tests/e2e/detectors/test_data/incorrect-return/0.8.10/incorrect_return.sol#4)
+
+C.bad0() (tests/e2e/detectors/test_data/incorrect-return/0.8.10/incorrect_return.sol#8-11) calls C.internal_return() (tests/e2e/detectors/test_data/incorrect-return/0.8.10/incorrect_return.sol#2-6) which halt the execution return(uint256,uint256)(5,6) (tests/e2e/detectors/test_data/incorrect-return/0.8.10/incorrect_return.sol#4)
+

tests/e2e/detectors/snapshots/detectors__detector_ReturnInsteadOfLeave_0_8_10_incorrect_return_sol__0.txt

@@ -0,0 +1,2 @@
+C.f() (tests/e2e/detectors/test_data/return-leave/0.8.10/incorrect_return.sol#3-7) contains an incorrect call to return: return(uint256,uint256)(5,6) (tests/e2e/detectors/test_data/return-leave/0.8.10/incorrect_return.sol#5)
+

tests/e2e/detectors/test_data/incorrect-return/0.8.10/incorrect_return.sol

@@ -0,0 +1,36 @@
+contract C {
+    function internal_return() internal{
+        assembly {
+            return (5, 6)
+        }
+    }
+
+    function bad0() public returns (bool){
+        internal_return();
+        return true;
+    }
+
+    function indirect2() internal {
+        internal_return();
+    }
+
+    function indirect() internal {
+        indirect2();
+    }
+
+    function bad1() public returns (bool){
+        indirect();
+        return true;
+    }
+
+    function good0() public{
+        // Dont report if there is no following operation
+        internal_return();
+    }
+
+    function good1() public returns (uint a, uint b){
+        assembly {
+            return (5, 6)
+        }
+    }
+}

tests/e2e/detectors/test_data/return-leave/0.8.10/incorrect_return.sol

@@ -0,0 +1,8 @@
+contract C {
+
+    function f() internal returns (uint a, uint b){
+        assembly {
+            return (5, 6)
+        }
+    }
+}

tests/e2e/detectors/test_detectors.py

@@ -1654,6 +1654,16 @@ ALL_TESTS = [
         "encode_packed_collision.sol",
         "0.7.6",
     ),
+    Test(
+        all_detectors.IncorrectReturn,
+        "incorrect_return.sol",
+        "0.8.10",
+    ),
+    Test(
+        all_detectors.ReturnInsteadOfLeave,
+        "incorrect_return.sol",
+        "0.8.10",
+    ),
 ]
 
 GENERIC_PATH = "/GENERIC_PATH"

tests/e2e/solc_parsing/test_ast_parsing.py

@@ -4,13 +4,13 @@ import re
 import sys
 from pathlib import Path
 from typing import List, Dict, Tuple
-from packaging.version import parse as parse_version
-import pytest
+
+from crytic_compile import CryticCompile, save_to_zip
+from crytic_compile.utils.zip import load_from_zip
 from deepdiff import DeepDiff
+from packaging.version import parse as parse_version
 from solc_select.solc_select import install_artifacts as install_solc_versions
 from solc_select.solc_select import installed_versions as get_installed_solc_versions
-from crytic_compile import CryticCompile, save_to_zip
-from crytic_compile.utils.zip import load_from_zip
 
 from slither import Slither
 from slither.printers.guidance.echidna import Echidna