Merge branch 'dev' of github.com:crytic/slither into dev

.github/workflows/ci.yml

@@ -29,7 +29,7 @@ jobs:
                # "embark",
                "erc",
                # "etherlime",
-               # "etherscan"
+               "etherscan",
                "find_paths",
                "flat",
                "kspec",

CODEOWNERS

@@ -0,0 +1,4 @@
+*                           @montyly @0xalpharush @smonicas
+/slither/tools/read_storage @0xalpharush
+/slither/slithir/           @montyly
+/slither/analyses/          @montyly

CONTRIBUTING.md

@@ -64,7 +64,10 @@ For each new detector, at least one regression tests must be present.
   - If updating an existing detector, identify the respective json artifacts and then delete them, or run `python ./tests/test_detectors.py --overwrite` instead.
 - Run `pytest ./tests/test_detectors.py` and check that everything worked.
 
-To see the tests coverage, run `pytest  tests/test_detectors.py  --cov=slither/detectors --cov-branch --cov-report html`
+To see the tests coverage, run `pytest  tests/test_detectors.py  --cov=slither/detectors --cov-branch --cov-report html`.
+To run tests for a specific detector, run `pytest tests/test_detectors.py -k ReentrancyReadBeforeWritten` (the detector's class name is the argument).
+To run tests for a specific version, run `pytest tests/test_detectors.py -k 0.7.6`.
+The IDs of tests can be inspected using `pytest tests/test_detectors.py --collect-only`.
 
 ### Parser tests
 - Create a test in `tests/ast-parsing`
@@ -73,6 +76,10 @@ To see the tests coverage, run `pytest  tests/test_detectors.py  --cov=slither/d
 - Run `pytest ./tests/test_ast_parsing.py` and check that everything worked.
 
 To see the tests coverage, run `pytest  tests/test_ast_parsing.py  --cov=slither/solc_parsing --cov-branch --cov-report html`
+To run tests for a specific test case, run `pytest tests/test_ast_parsing.py -k user_defined_value_type`  (the filename is the argument).
+To run tests for a specific version, run `pytest tests/test_ast_parsing.py -k 0.8.12`.
+To run tests for a specific compiler json format, run `pytest tests/test_ast_parsing.py -k legacy` (can be legacy or compact).
+The IDs of tests can be inspected using ``pytest tests/test_ast_parsing.py --collect-only`.
 
 ### Synchronization with crytic-compile
 By default, `slither` follows either the latest version of crytic-compile in pip, or `crytic-compile@master` (look for dependencies in [`setup.py`](./setup.py). If crytic-compile development comes with breaking changes, the process to update `slither` is:

scripts/ci_test_etherscan.sh

@@ -5,15 +5,24 @@
 mkdir etherscan
 cd etherscan || exit 255
 
-if ! slither 0x7F37f78cBD74481E593F9C737776F7113d76B315 --etherscan-apikey "$GITHUB_ETHERSCAN"; then
-    echo "Etherscan test failed"
+echo "::group::Etherscan mainnet"
+if ! slither 0x7F37f78cBD74481E593F9C737776F7113d76B315 --etherscan-apikey "$GITHUB_ETHERSCAN" --no-fail-pedantic; then
+    echo "Etherscan mainnet test failed"
     exit 1
 fi
+echo "::endgroup::"
 
-if ! slither rinkeby:0xFe05820C5A92D9bc906D4A46F662dbeba794d3b7 --etherscan-apikey "$GITHUB_ETHERSCAN"; then
-    echo "Etherscan test failed"
+# Perform a small sleep when API key is not available (e.g. on PR CI from external contributor)
+if [ "$GITHUB_ETHERSCAN" = "" ]; then
+    sleep $(( ( RANDOM % 5 )  + 1 ))s
+fi
+
+echo "::group::Etherscan rinkeby"
+if ! slither rinkeby:0xFe05820C5A92D9bc906D4A46F662dbeba794d3b7 --etherscan-apikey "$GITHUB_ETHERSCAN" --no-fail-pedantic; then
+    echo "Etherscan rinkeby test failed"
     exit 1
 fi
+echo "::endgroup::"
 
 exit 0
 

slither/solc_parsing/declarations/contract.py

@@ -616,23 +616,41 @@ class ContractSolc(CallerContextExpression):
 
     def _analyze_function_list(self, function_list: List, type_name: Type):
         for f in function_list:
-            function_name = f["function"]["name"]
-            if function_name.find(".") != -1:
-                # Library function
-                self._analyze_library_function(function_name, type_name)
-            else:
+            full_name_split = f["function"]["name"].split(".")
+            if len(full_name_split) == 1:
                 # Top level function
+                function_name = full_name_split[0]
+                self._analyze_top_level_function(function_name, type_name)
+            elif len(full_name_split) == 2:
+                # It can be a top level function behind an aliased import
+                # or a library function
+                first_part = full_name_split[0]
+                function_name = full_name_split[1]
+                self._check_aliased_import(first_part, function_name, type_name)
+            else:
+                # MyImport.MyLib.a we don't care of the alias
+                library_name = full_name_split[1]
+                function_name = full_name_split[2]
+                self._analyze_library_function(library_name, function_name, type_name)
+
+    def _check_aliased_import(self, first_part: str, function_name: str, type_name: Type):
+        # We check if the first part appear as alias for an import
+        # if it is then function_name must be a top level function
+        # otherwise it's a library function
+        for i in self._contract.file_scope.imports:
+            if i.alias == first_part:
+                self._analyze_top_level_function(function_name, type_name)
+                return
+        self._analyze_library_function(first_part, function_name, type_name)
+
+    def _analyze_top_level_function(self, function_name: str, type_name: Type):
         for tl_function in self.compilation_unit.functions_top_level:
             if tl_function.name == function_name:
                 self._contract.using_for[type_name].append(tl_function)
 
-    def _analyze_library_function(self, function_name: str, type_name: Type) -> None:
-        function_name_split = function_name.split(".")
-        # TODO this doesn't handle the case if there is an import with an alias
-        # e.g. MyImport.MyLib.a
-        if len(function_name_split) == 2:
-            library_name = function_name_split[0]
-            function_name = function_name_split[1]
+    def _analyze_library_function(
+        self, library_name: str, function_name: str, type_name: Type
+    ) -> None:
         # Get the library function
         found = False
         for c in self.compilation_unit.contracts:
@@ -645,10 +663,8 @@ class ContractSolc(CallerContextExpression):
                         found = True
                         break
         if not found:
-                self.log_incorrect_parsing(f"Library function not found {function_name}")
-        else:
             self.log_incorrect_parsing(
-                f"Expected library function instead received {function_name}"
+                f"Contract level using for: Library {library_name} - function {function_name} not found"
             )
 
     def analyze_enums(self):

slither/solc_parsing/declarations/using_for_top_level.py

@@ -2,19 +2,19 @@
     Using For Top Level module
 """
 import logging
-from typing import TYPE_CHECKING, Dict, Union, Any
+from typing import TYPE_CHECKING, Dict, Union
 
 from slither.core.compilation_unit import SlitherCompilationUnit
-from slither.core.declarations.using_for_top_level import UsingForTopLevel
-from slither.core.scope.scope import FileScope
-from slither.core.solidity_types import TypeAliasTopLevel
 from slither.core.declarations import (
     StructureTopLevel,
     EnumTopLevel,
 )
+from slither.core.declarations.using_for_top_level import UsingForTopLevel
+from slither.core.scope.scope import FileScope
+from slither.core.solidity_types import TypeAliasTopLevel
+from slither.core.solidity_types.user_defined_type import UserDefinedType
 from slither.solc_parsing.declarations.caller_context import CallerContextExpression
 from slither.solc_parsing.solidity_types.type_parsing import parse_type
-from slither.core.solidity_types.user_defined_type import UserDefinedType
 
 if TYPE_CHECKING:
     from slither.solc_parsing.slither_compilation_unit_solc import SlitherCompilationUnitSolc
@@ -58,20 +58,34 @@ class UsingForTopLevelSolc(CallerContextExpression):  # pylint: disable=too-few-
                 full_name_split = f["function"]["name"].split(".")
                 if len(full_name_split) == 1:
                     # Top level function
-                    function_name = full_name_split[0]
+                    function_name: str = full_name_split[0]
                     self._analyze_top_level_function(function_name, type_name)
                 elif len(full_name_split) == 2:
-                    # Library function
-                    library_name = full_name_split[0]
+                    # It can be a top level function behind an aliased import
+                    # or a library function
+                    first_part = full_name_split[0]
                     function_name = full_name_split[1]
-                    self._analyze_library_function(function_name, library_name, type_name)
+                    self._check_aliased_import(first_part, function_name, type_name)
                 else:
-                    # probably case if there is an import with an alias we don't handle it for now
-                    # e.g. MyImport.MyLib.a
-                    LOGGER.warning(
-                        f"Using for directive for function {f['function']['name']} not supported"
-                    )
-                    continue
+                    # MyImport.MyLib.a we don't care of the alias
+                    library_name_str = full_name_split[1]
+                    function_name = full_name_split[2]
+                    self._analyze_library_function(library_name_str, function_name, type_name)
+
+    def _check_aliased_import(
+        self,
+        first_part: str,
+        function_name: str,
+        type_name: Union[TypeAliasTopLevel, UserDefinedType],
+    ):
+        # We check if the first part appear as alias for an import
+        # if it is then function_name must be a top level function
+        # otherwise it's a library function
+        for i in self._using_for.file_scope.imports:
+            if i.alias == first_part:
+                self._analyze_top_level_function(function_name, type_name)
+                return
+        self._analyze_library_function(first_part, function_name, type_name)
 
     def _analyze_top_level_function(
         self, function_name: str, type_name: Union[TypeAliasTopLevel, UserDefinedType]
@@ -84,8 +98,8 @@ class UsingForTopLevelSolc(CallerContextExpression):  # pylint: disable=too-few-
 
     def _analyze_library_function(
         self,
-        function_name: str,
         library_name: str,
+        function_name: str,
         type_name: Union[TypeAliasTopLevel, UserDefinedType],
     ) -> None:
         found = False
@@ -100,7 +114,9 @@ class UsingForTopLevelSolc(CallerContextExpression):  # pylint: disable=too-few-
                         found = True
                         break
         if not found:
-            LOGGER.warning(f"Library {library_name} - function {function_name} not found")
+            LOGGER.warning(
+                f"Top level using for: Library {library_name} - function {function_name} not found"
+            )
 
     def _propagate_global(self, type_name: Union[TypeAliasTopLevel, UserDefinedType]) -> None:
         if self._global:
@@ -116,9 +132,7 @@ class UsingForTopLevelSolc(CallerContextExpression):  # pylint: disable=too-few-
                         f"Error when propagating global using for {type_name} {type(type_name)}"
                     )
 
-    def _propagate_global_UserDefinedType(
-        self, scope: Dict[Any, FileScope], type_name: UserDefinedType
-    ):
+    def _propagate_global_UserDefinedType(self, scope: FileScope, type_name: UserDefinedType):
         underlying = type_name.type
         if isinstance(underlying, StructureTopLevel):
             for struct in scope.structures.values():

slither/solc_parsing/slither_compilation_unit_solc.py

@@ -512,7 +512,7 @@ Please rename it, this name is reserved for Slither's internals"""
         self._analyze_third_part(contracts_to_be_analyzed, libraries)
         [c.set_is_analyzed(False) for c in self._underlying_contract_to_parser.values()]
 
-        self._analyze_using_for(contracts_to_be_analyzed)
+        self._analyze_using_for(contracts_to_be_analyzed, libraries)
 
         self._parsed = True
 
@@ -624,9 +624,14 @@ Please rename it, this name is reserved for Slither's internals"""
             else:
                 contracts_to_be_analyzed += [contract]
 
-    def _analyze_using_for(self, contracts_to_be_analyzed: List[ContractSolc]):
+    def _analyze_using_for(
+        self, contracts_to_be_analyzed: List[ContractSolc], libraries: List[ContractSolc]
+    ):
         self._analyze_top_level_using_for()
 
+        for lib in libraries:
+            lib.analyze_using_for()
+
         while contracts_to_be_analyzed:
             contract = contracts_to_be_analyzed[0]
 

tests/ast-parsing/expected/using-for-alias-contract-0.8.0.sol-0.8.15-compact.json

@@ -0,0 +1,9 @@
+{
+  "C": {
+    "topLevel(uint256)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: EXPRESSION 1\n\"];\n}\n",
+    "libCall(uint256)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: EXPRESSION 1\n\"];\n}\n"
+  },
+  "Lib": {
+    "b(uint256)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: RETURN 1\n\"];\n}\n"
+  }
+}

tests/ast-parsing/expected/using-for-alias-top-level-0.8.0.sol-0.8.15-compact.json

@@ -0,0 +1,9 @@
+{
+  "Lib": {
+    "b(uint256)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: RETURN 1\n\"];\n}\n"
+  },
+  "C": {
+    "topLevel(uint256)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: EXPRESSION 1\n\"];\n}\n",
+    "libCall(uint256)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: EXPRESSION 1\n\"];\n}\n"
+  }
+}

tests/ast-parsing/expected/using-for-in-library-0.8.0.sol-0.8.15-compact.json

@@ -0,0 +1,8 @@
+{
+  "A": {
+    "a(uint256)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: RETURN 1\n\"];\n}\n"
+  },
+  "B": {
+    "b(uint256)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: RETURN 1\n\"];\n}\n"
+  }
+}

tests/ast-parsing/using-for-alias-contract-0.8.0.sol

@@ -0,0 +1,14 @@
+import "./using-for-alias-dep1.sol";
+
+contract C {
+    using {T3.a, T3.Lib.b} for uint256;
+
+    function topLevel(uint256 value) public {
+        value.a();
+    }
+
+    function libCall(uint256 value) public {
+        value.b();
+    }
+
+}

tests/ast-parsing/using-for-alias-dep1.sol

@@ -0,0 +1,11 @@
+import "./using-for-alias-dep2.sol" as T3;
+
+function b(uint256 value) returns(bool) {
+    return true;
+}
+
+library Lib {
+    function a(uint256 value) public returns(bool) {
+        return true;
+    }
+}

tests/ast-parsing/using-for-alias-dep2.sol

@@ -0,0 +1,9 @@
+function a(uint256 value) returns(bool) {
+    return true;
+}
+
+library Lib {
+    function b(uint256 value) public returns(bool) {
+        return true;
+    }
+}

tests/ast-parsing/using-for-alias-top-level-0.8.0.sol

@@ -0,0 +1,15 @@
+import "./using-for-alias-dep1.sol";
+
+using {T3.a, T3.Lib.b} for uint256;
+
+contract C {
+
+    function topLevel(uint256 value) public {
+        value.a();
+    }
+
+    function libCall(uint256 value) public {
+        value.b();
+    }
+
+}

tests/ast-parsing/using-for-in-library-0.8.0.sol

@@ -0,0 +1,14 @@
+
+library A {
+    using B for uint256;
+    
+    function a(uint256 v) public view returns (uint) {
+        return v.b();
+    }
+}
+
+library B {
+    function b(uint256 v) public view returns (uint) {
+        return 1;
+    }
+}

tests/test_ast_parsing.py

@@ -428,6 +428,9 @@ ALL_TESTS = [
     Test("using-for-2-0.8.0.sol", ["0.8.15"]),
     Test("using-for-3-0.8.0.sol", ["0.8.15"]),
     Test("using-for-4-0.8.0.sol", ["0.8.15"]),
+    Test("using-for-in-library-0.8.0.sol", ["0.8.15"]),
+    Test("using-for-alias-contract-0.8.0.sol", ["0.8.15"]),
+    Test("using-for-alias-top-level-0.8.0.sol", ["0.8.15"]),
     Test("using-for-functions-list-1-0.8.0.sol", ["0.8.15"]),
     Test("using-for-functions-list-2-0.8.0.sol", ["0.8.15"]),
     Test("using-for-functions-list-3-0.8.0.sol", ["0.8.15"]),
@@ -443,20 +446,21 @@ except OSError:
     pass
 
 
-@pytest.mark.parametrize("test_item", ALL_TESTS, ids=lambda x: x.test_file)
-def test_parsing(test_item: Test):
-    flavors = ["compact"]
-    if not test_item.disable_legacy:
-        flavors += ["legacy"]
+def pytest_generate_tests(metafunc):
+    test_cases = []
+    for test_item in ALL_TESTS:
         for version, flavor in test_item.versions_with_flavors:
-        test_file = os.path.join(
-            TEST_ROOT, "compile", f"{test_item.test_file}-{version}-{flavor}.zip"
-        )
-        expected_file = os.path.join(
-            TEST_ROOT, "expected", f"{test_item.test_file}-{version}-{flavor}.json"
-        )
+            test_cases.append((test_item.test_file, version, flavor))
+    metafunc.parametrize("test_file, version, flavor", test_cases)
 
-        cc = load_from_zip(test_file)[0]
+
+class TestASTParsing:
+    # pylint: disable=no-self-use
+    def test_parsing(self, test_file, version, flavor):
+        actual = os.path.join(TEST_ROOT, "compile", f"{test_file}-{version}-{flavor}.zip")
+        expected = os.path.join(TEST_ROOT, "expected", f"{test_file}-{version}-{flavor}.json")
+
+        cc = load_from_zip(actual)[0]
 
         sl = Slither(
             cc,
@@ -468,26 +472,25 @@ def test_parsing(test_item: Test):
         actual = generate_output(sl)
 
         try:
-            with open(expected_file, "r", encoding="utf8") as f:
+            with open(expected, "r", encoding="utf8") as f:
                 expected = json.load(f)
         except OSError:
             pytest.xfail("the file for this test was not generated")
             raise
 
         diff = DeepDiff(expected, actual, ignore_order=True, verbose_level=2, view="tree")
-
         if diff:
             for change in diff.get("values_changed", []):
                 path_list = re.findall(r"\['(.*?)'\]", change.path())
                 path = "_".join(path_list)
                 with open(
-                    f"test_artifacts/{test_item.test_file}_{path}_expected.dot",
+                    f"test_artifacts/{test_file}_{path}_expected.dot",
                     "w",
                     encoding="utf8",
                 ) as f:
                     f.write(change.t1)
                 with open(
-                    f"test_artifacts/{test_item.test_file}_{version}_{flavor}_{path}_actual.dot",
+                    f"test_artifacts/{test_file}_{version}_{flavor}_{path}_actual.dot",
                     "w",
                     encoding="utf8",
                 ) as f:

tests/test_detectors.py

@@ -52,7 +52,7 @@ def set_solc(test_item: Test):  # pylint: disable=too-many-lines
 
 
 def id_test(test_item: Test):
-    return f"{test_item.detector}: {test_item.solc_ver}/{test_item.test_file}"
+    return f"{test_item.detector.__name__}-{test_item.solc_ver}-{test_item.test_file}"
 
 
 ALL_TEST_OBJECTS = [

tests/test_features.py

@@ -7,7 +7,7 @@ from solc_select import solc_select
 from slither import Slither
 from slither.detectors import all_detectors
 from slither.detectors.abstract_detector import AbstractDetector
-from slither.slithir.operations import LibraryCall
+from slither.slithir.operations import LibraryCall, InternalCall
 
 
 def _run_all_detectors(slither: Slither) -> None:
@@ -92,3 +92,50 @@ def test_using_for_top_level_implicit_conversion() -> None:
         if isinstance(ir, LibraryCall) and ir.destination == "Lib" and ir.function_name == "f":
             return
     assert False
+
+
+def test_using_for_alias_top_level() -> None:
+    solc_select.switch_global_version("0.8.15", always_install=True)
+    slither = Slither("./tests/ast-parsing/using-for-alias-top-level-0.8.0.sol")
+    contract_c = slither.get_contract_from_name("C")[0]
+    libCall = contract_c.get_function_from_full_name("libCall(uint256)")
+    ok = False
+    for ir in libCall.all_slithir_operations():
+        if isinstance(ir, LibraryCall) and ir.destination == "Lib" and ir.function_name == "b":
+            ok = True
+    if not ok:
+        assert False
+    topLevelCall = contract_c.get_function_from_full_name("topLevel(uint256)")
+    for ir in topLevelCall.all_slithir_operations():
+        if isinstance(ir, InternalCall) and ir.function_name == "a":
+            return
+    assert False
+
+
+def test_using_for_alias_contract() -> None:
+    solc_select.switch_global_version("0.8.15", always_install=True)
+    slither = Slither("./tests/ast-parsing/using-for-alias-contract-0.8.0.sol")
+    contract_c = slither.get_contract_from_name("C")[0]
+    libCall = contract_c.get_function_from_full_name("libCall(uint256)")
+    ok = False
+    for ir in libCall.all_slithir_operations():
+        if isinstance(ir, LibraryCall) and ir.destination == "Lib" and ir.function_name == "b":
+            ok = True
+    if not ok:
+        assert False
+    topLevelCall = contract_c.get_function_from_full_name("topLevel(uint256)")
+    for ir in topLevelCall.all_slithir_operations():
+        if isinstance(ir, InternalCall) and ir.function_name == "a":
+            return
+    assert False
+
+
+def test_using_for_in_library() -> None:
+    solc_select.switch_global_version("0.8.15", always_install=True)
+    slither = Slither("./tests/ast-parsing/using-for-in-library-0.8.0.sol")
+    contract_c = slither.get_contract_from_name("A")[0]
+    libCall = contract_c.get_function_from_full_name("a(uint256)")
+    for ir in libCall.all_slithir_operations():
+        if isinstance(ir, LibraryCall) and ir.destination == "B" and ir.function_name == "b":
+            return
+    assert False