Merge pull request #1625 from crytic/using-for-global-lib-collision

Fix using for global function name collision

slither/slithir/convert.py

@@ -525,9 +525,7 @@ def _convert_type_contract(ir: Member) -> Assignment:
     raise SlithIRError(f"type({contract.name}).{ir.variable_right} is unknown")
 
 
-def propagate_types(
-    ir: slither.slithir.operations.operation.Operation, node: "Node"
-):  # pylint: disable=too-many-locals
+def propagate_types(ir: Operation, node: "Node"):  # pylint: disable=too-many-locals
     # propagate the type
     node_function = node.function
     using_for = (

slither/solc_parsing/declarations/using_for_top_level.py

@@ -49,7 +49,7 @@ class UsingForTopLevelSolc(CallerContextExpression):  # pylint: disable=too-few-
         type_name = parse_type(self._type_name, self)
         self._using_for.using_for[type_name] = []
 
-        if self._library_name is not None:
+        if self._library_name:
             library_name = parse_type(self._library_name, self)
             self._using_for.using_for[type_name].append(library_name)
             self._propagate_global(type_name)
@@ -90,8 +90,13 @@ class UsingForTopLevelSolc(CallerContextExpression):  # pylint: disable=too-few-
     def _analyze_top_level_function(
         self, function_name: str, type_name: Union[TypeAliasTopLevel, UserDefinedType]
     ) -> None:
-        for tl_function in self.compilation_unit.functions_top_level:
-            if tl_function.name == function_name:
+        for tl_function in self._using_for.file_scope.functions:
+            # The library function is bound to the first parameter's type
+            if (
+                tl_function.name == function_name
+                and tl_function.parameters
+                and type_name == tl_function.parameters[0].type
+            ):
                 self._using_for.using_for[type_name].append(tl_function)
                 self._propagate_global(type_name)
                 break
@@ -108,7 +113,12 @@ class UsingForTopLevelSolc(CallerContextExpression):  # pylint: disable=too-few-
                 break
             if c.name == library_name:
                 for cf in c.functions:
-                    if cf.name == function_name:
+                    # The library function is bound to the first parameter's type
+                    if (
+                        cf.name == function_name
+                        and cf.parameters
+                        and type_name == cf.parameters[0].type
+                    ):
                         self._using_for.using_for[type_name].append(cf)
                         self._propagate_global(type_name)
                         found = True

tests/conftest.py

@@ -0,0 +1,59 @@
+import os
+from contextlib import contextmanager
+from pathlib import Path
+from typing import Optional
+
+import pytest
+from crytic_compile import CryticCompile
+from crytic_compile.platform.solc_standard_json import SolcStandardJson
+from solc_select import solc_select
+
+from slither import Slither
+
+
+@contextmanager
+def _select_solc_version(version: Optional[str]):
+    """Selects solc version to use for running tests.
+    If no version is provided, latest is used."""
+    if not version:
+        # This sorts the versions numerically
+        vers = sorted(
+            map(
+                lambda x: (int(x[0]), int(x[1]), int(x[2])),
+                map(lambda x: x.split(".", 3), solc_select.installed_versions()),
+            )
+        )
+        ver = list(vers)[-1]
+        version = ".".join(map(str, ver))
+    env = dict(os.environ)
+    env_restore = dict(env)
+    env["SOLC_VERSION"] = version
+    os.environ.clear()
+    os.environ.update(env)
+
+    yield version
+
+    os.environ.clear()
+    os.environ.update(env_restore)
+
+
+@pytest.fixture(name="select_solc_version")
+def fixture_select_solc_version():
+    return _select_solc_version
+
+
+@pytest.fixture
+def slither_from_dir(select_solc_version):
+    @contextmanager
+    def _slither_from_dir(directory: str, solc_version: Optional[str] = None):
+        """Yields a Slither instance using solidity files in directory and solc_version.
+        Temporarily changes the solc-version temporary to solc_version.
+        """
+        standard_json = SolcStandardJson()
+        for source_file in Path(directory).rglob("*.sol"):
+            standard_json.add_source_file(Path(source_file).as_posix())
+        with select_solc_version(solc_version):
+            compilation = CryticCompile(standard_json)
+            yield Slither(compilation)
+
+    return _slither_from_dir

tests/test_features.py

@@ -8,7 +8,7 @@ from slither import Slither
 from slither.core.variables.state_variable import StateVariable
 from slither.detectors import all_detectors
 from slither.detectors.abstract_detector import AbstractDetector
-from slither.slithir.operations import LibraryCall, InternalCall
+from slither.slithir.operations import InternalCall, LibraryCall
 from slither.utils.arithmetic import unchecked_arithemtic_usage
 
 
@@ -160,3 +160,8 @@ def test_arithmetic_usage() -> None:
     assert {
         f.source_mapping.content_hash for f in unchecked_arithemtic_usage(slither.contracts[0])
     } == {"2b4bc73cf59d486dd9043e840b5028b679354dd9", "e4ecd4d0fda7e762d29aceb8425f2c5d4d0bf962"}
+
+
+def test_using_for_global_collision(slither_from_dir) -> None:
+    with slither_from_dir("./tests/using-for-global-collision") as sl:
+        _run_all_detectors(sl)

tests/using-for-global-collision/src/MyTypeA.sol

@@ -0,0 +1,2 @@
+import "./MyTypeA/Type.sol";
+import "./MyTypeA/Math.sol";

tests/using-for-global-collision/src/MyTypeA/Casting.sol

@@ -0,0 +1,4 @@
+import "./Type.sol";
+function unwrap(MyTypeA a) pure returns (int256) {
+    return MyTypeA.unwrap(a);
+}

tests/using-for-global-collision/src/MyTypeA/Math.sol

@@ -0,0 +1,5 @@
+import "./Type.sol";
+
+function mul(MyTypeA a, MyTypeA b) pure returns (MyTypeA) {
+    return MyTypeA.wrap(MyTypeA.unwrap(a) * MyTypeA.unwrap(b));
+}

tests/using-for-global-collision/src/MyTypeA/Type.sol

@@ -0,0 +1,6 @@
+import "./Casting.sol" as C;
+import "./Math.sol" as M;
+
+type MyTypeA is int256;
+
+using {M.mul, C.unwrap} for MyTypeA global;

tests/using-for-global-collision/src/MyTypeB.sol

@@ -0,0 +1,2 @@
+import "./MyTypeB/Type.sol";
+import "./MyTypeB/Math.sol";

tests/using-for-global-collision/src/MyTypeB/Casting.sol

@@ -0,0 +1,4 @@
+import "./Type.sol";
+function unwrap(MyTypeB a) pure returns (uint256) {
+    return MyTypeB.unwrap(a);
+}

tests/using-for-global-collision/src/MyTypeB/Math.sol

@@ -0,0 +1,6 @@
+import "./Type.sol";
+
+function mul(MyTypeB a, MyTypeB b) pure returns (MyTypeB) {
+    return MyTypeB.wrap(MyTypeB.unwrap(a) * MyTypeB.unwrap(b));
+}
+

tests/using-for-global-collision/src/MyTypeB/Type.sol

@@ -0,0 +1,6 @@
+import "./Casting.sol" as C;
+import "./Math.sol" as M;
+
+type MyTypeB is uint256;
+
+using {M.mul, C.unwrap} for MyTypeB global;

tests/using-for-global-collision/src/Test.sol

@@ -0,0 +1,7 @@
+import "./MyTypeB.sol";
+
+contract UsingForGlobalTopLevelCollision {
+    function mulAndUnwrap(MyTypeB x, MyTypeB y) external pure returns (uint256 z) {
+        z = x.mul(y).unwrap();
+    }
+}