Merge pull request #1653 from bart1e/detector_for_invalid_using_for

Detector for invalid using for at the top level

slither/detectors/all_detectors.py

@@ -89,4 +89,5 @@
 from .functions.permit_domain_signature_collision import DomainSeparatorCollision
 from .functions.codex import Codex
 from .functions.cyclomatic_complexity import CyclomaticComplexity
+from .statements.incorrect_using_for import IncorrectUsingFor
 from .operations.encode_packed import EncodePackedCollision

slither/detectors/statements/incorrect_using_for.py

@@ -0,0 +1,223 @@
+from typing import List
+
+from slither.core.declarations import Contract, Structure, Enum
+from slither.core.declarations.using_for_top_level import UsingForTopLevel
+from slither.core.solidity_types import (
+    UserDefinedType,
+    Type,
+    ElementaryType,
+    TypeAlias,
+    MappingType,
+    ArrayType,
+)
+from slither.core.solidity_types.elementary_type import Uint, Int, Byte
+from slither.detectors.abstract_detector import (
+    AbstractDetector,
+    DetectorClassification,
+    DETECTOR_INFO,
+)
+from slither.utils.output import Output
+
+
+def _is_correctly_used(type_: Type, library: Contract) -> bool:
+    """
+    Checks if a `using library for type_` statement is used correctly (that is, does library contain any function
+    with type_ as the first argument).
+    """
+    for f in library.functions:
+        if len(f.parameters) == 0:
+            continue
+        if f.parameters[0].type and not _implicitly_convertible_to(type_, f.parameters[0].type):
+            continue
+        return True
+    return False
+
+
+def _implicitly_convertible_to(type1: Type, type2: Type) -> bool:
+    """
+    Returns True if type1 may be implicitly converted to type2.
+    """
+    if isinstance(type1, TypeAlias) or isinstance(type2, TypeAlias):
+        if isinstance(type1, TypeAlias) and isinstance(type2, TypeAlias):
+            return type1.type == type2.type
+        return False
+
+    if isinstance(type1, UserDefinedType) and isinstance(type2, UserDefinedType):
+        if isinstance(type1.type, Contract) and isinstance(type2.type, Contract):
+            return _implicitly_convertible_to_for_contracts(type1.type, type2.type)
+
+        if isinstance(type1.type, Structure) and isinstance(type2.type, Structure):
+            return type1.type.canonical_name == type2.type.canonical_name
+
+        if isinstance(type1.type, Enum) and isinstance(type2.type, Enum):
+            return type1.type.canonical_name == type2.type.canonical_name
+
+    if isinstance(type1, ElementaryType) and isinstance(type2, ElementaryType):
+        return _implicitly_convertible_to_for_elementary_types(type1, type2)
+
+    if isinstance(type1, MappingType) and isinstance(type2, MappingType):
+        return _implicitly_convertible_to_for_mappings(type1, type2)
+
+    if isinstance(type1, ArrayType) and isinstance(type2, ArrayType):
+        return _implicitly_convertible_to_for_arrays(type1, type2)
+
+    return False
+
+
+def _implicitly_convertible_to_for_arrays(type1: ArrayType, type2: ArrayType) -> bool:
+    """
+    Returns True if type1 may be implicitly converted to type2.
+    """
+    return _implicitly_convertible_to(type1.type, type2.type)
+
+
+def _implicitly_convertible_to_for_mappings(type1: MappingType, type2: MappingType) -> bool:
+    """
+    Returns True if type1 may be implicitly converted to type2.
+    """
+    return type1.type_from == type2.type_from and type1.type_to == type2.type_to
+
+
+def _implicitly_convertible_to_for_elementary_types(
+    type1: ElementaryType, type2: ElementaryType
+) -> bool:
+    """
+    Returns True if type1 may be implicitly converted to type2.
+    """
+    if type1.type == "bool" and type2.type == "bool":
+        return True
+    if type1.type == "string" and type2.type == "string":
+        return True
+    if type1.type == "bytes" and type2.type == "bytes":
+        return True
+    if type1.type == "address" and type2.type == "address":
+        return _implicitly_convertible_to_for_addresses(type1, type2)
+    if type1.type in Uint and type2.type in Uint:
+        return _implicitly_convertible_to_for_uints(type1, type2)
+    if type1.type in Int and type2.type in Int:
+        return _implicitly_convertible_to_for_ints(type1, type2)
+    if (
+        type1.type != "bytes"
+        and type2.type != "bytes"
+        and type1.type in Byte
+        and type2.type in Byte
+    ):
+        return _implicitly_convertible_to_for_bytes(type1, type2)
+    return False
+
+
+def _implicitly_convertible_to_for_bytes(type1: ElementaryType, type2: ElementaryType) -> bool:
+    """
+    Returns True if type1 may be implicitly converted to type2 assuming they are both bytes.
+    """
+    assert type1.type in Byte and type2.type in Byte
+    assert type1.size is not None
+    assert type2.size is not None
+
+    return type1.size <= type2.size
+
+
+def _implicitly_convertible_to_for_addresses(type1: ElementaryType, type2: ElementaryType) -> bool:
+    """
+    Returns True if type1 may be implicitly converted to type2 assuming they are both addresses.
+    """
+    assert type1.type == "address" and type2.type == "address"
+    # payable attribute to be implemented; for now, always return True
+    return True
+
+
+def _implicitly_convertible_to_for_ints(type1: ElementaryType, type2: ElementaryType) -> bool:
+    """
+    Returns True if type1 may be implicitly converted to type2 assuming they are both ints.
+    """
+    assert type1.type in Int and type2.type in Int
+    assert type1.size is not None
+    assert type2.size is not None
+
+    return type1.size <= type2.size
+
+
+def _implicitly_convertible_to_for_uints(type1: ElementaryType, type2: ElementaryType) -> bool:
+    """
+    Returns True if type1 may be implicitly converted to type2 assuming they are both uints.
+    """
+    assert type1.type in Uint and type2.type in Uint
+    assert type1.size is not None
+    assert type2.size is not None
+
+    return type1.size <= type2.size
+
+
+def _implicitly_convertible_to_for_contracts(contract1: Contract, contract2: Contract) -> bool:
+    """
+    Returns True if contract1 may be implicitly converted to contract2.
+    """
+    return contract1 == contract2 or contract2 in contract1.inheritance
+
+
+class IncorrectUsingFor(AbstractDetector):
+    """
+    Detector for incorrect using-for statement usage.
+    """
+
+    ARGUMENT = "incorrect-using-for"
+    HELP = "Detects using-for statement usage when no function from a given library matches a given type"
+    IMPACT = DetectorClassification.INFORMATIONAL
+    CONFIDENCE = DetectorClassification.HIGH
+
+    WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-using-for-usage"
+
+    WIKI_TITLE = "Incorrect usage of using-for statement"
+    WIKI_DESCRIPTION = (
+        "In Solidity, it is possible to use libraries for certain types, by the `using-for` statement "
+        "(`using <library> for <type>`). However, the Solidity compiler doesn't check whether a given "
+        "library has at least one function matching a given type. If it doesn't, such a statement has "
+        "no effect and may be confusing. "
+    )
+
+    # region wiki_exploit_scenario
+    WIKI_EXPLOIT_SCENARIO = """
+    ```solidity
+    library L {
+        function f(bool) public pure {}
+    }
+    
+    using L for uint;
+    ```
+    Such a code will compile despite the fact that `L` has no function with `uint` as its first argument."""
+    # endregion wiki_exploit_scenario
+    WIKI_RECOMMENDATION = (
+        "Make sure that the libraries used in `using-for` statements have at least one function "
+        "matching a type used in these statements. "
+    )
+
+    def _append_result(
+        self, results: List[Output], uf: UsingForTopLevel, type_: Type, library: Contract
+    ) -> None:
+        info: DETECTOR_INFO = [
+            f"using-for statement at {uf.source_mapping} is incorrect - no matching function for {type_} found in ",
+            library,
+            ".\n",
+        ]
+        res = self.generate_result(info)
+        results.append(res)
+
+    def _detect(self) -> List[Output]:
+        results: List[Output] = []
+
+        for uf in self.compilation_unit.using_for_top_level:
+            # UsingForTopLevel.using_for is a dict with a single entry, which is mapped to a list of functions/libraries
+            # the following code extracts the type from using-for and skips using-for statements with functions
+            type_ = list(uf.using_for.keys())[0]
+            for lib_or_fcn in uf.using_for[type_]:
+                # checking for using-for with functions is already performed by the compiler; we only consider libraries
+                if isinstance(lib_or_fcn, UserDefinedType):
+                    lib_or_fcn_type = lib_or_fcn.type
+                    if (
+                        isinstance(type_, Type)
+                        and isinstance(lib_or_fcn_type, Contract)
+                        and not _is_correctly_used(type_, lib_or_fcn_type)
+                    ):
+                        self._append_result(results, uf, type_, lib_or_fcn_type)
+
+        return results

tests/e2e/detectors/snapshots/detectors__detector_IncorrectUsingFor_0_8_17_IncorrectUsingForTopLevel_sol__0.txt

@@ -0,0 +1,24 @@
+using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#84 is incorrect - no matching function for bytes17[] found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64).
+
+using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#85 is incorrect - no matching function for uint256 found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64).
+
+using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#90 is incorrect - no matching function for mapping(int256 => uint128) found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64).
+
+using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#86 is incorrect - no matching function for int256 found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64).
+
+using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#89 is incorrect - no matching function for E2 found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64).
+
+using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#93 is incorrect - no matching function for bytes[][] found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64).
+
+using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#92 is incorrect - no matching function for string[][] found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64).
+
+using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#91 is incorrect - no matching function for mapping(int128 => uint256) found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64).
+
+using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#87 is incorrect - no matching function for bytes18 found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64).
+
+using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#88 is incorrect - no matching function for S2 found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64).
+
+using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#83 is incorrect - no matching function for C3 found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64).
+
+using-for statement at tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#94 is incorrect - no matching function for custom_int found in L (tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol#48-64).
+

tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol

@@ -0,0 +1,94 @@
+pragma solidity 0.8.17;
+
+struct S1
+{
+    uint __;
+}
+
+struct S2
+{
+    uint128 __;
+}
+
+enum E1
+{
+    A,
+    B
+}
+
+enum E2
+{
+    A,
+    B
+}
+
+contract C0
+{
+
+}
+
+contract C1 is C0
+{
+
+}
+
+contract C2 is C1
+{
+
+}
+
+contract C3
+{
+
+}
+
+type custom_uint is uint248;
+type custom_int is int248;
+
+library L
+{
+    function f0(C0) public pure {}
+    function f1(bool) public pure {}
+    function f2(string memory) public pure {}
+    function f3(bytes memory) public pure {}
+    function f4(uint248) public pure {}
+    function f5(int248) public pure {}
+    function f6(address) public pure {}
+    function f7(bytes17) public pure {}
+    function f8(S1 memory) public pure {}
+    function f9(E1) public pure {}
+    function f10(mapping(int => uint) storage) public pure {}
+    function f11(string[] memory) public pure {}
+    function f12(bytes[][][] memory) public pure {}
+    function f13(custom_uint) public pure {}
+}
+
+// the following statements are correct
+using L for C2;
+using L for bool;
+using L for string;
+using L for bytes;
+using L for uint240;
+using L for int16;
+using L for address;
+using L for bytes16;
+using L for S1;
+using L for E1;
+using L for mapping(int => uint);
+using L for string[];
+using L for bytes[][][];
+using L for custom_uint;
+
+// the following statements are incorrect
+using L for C3;
+using L for bytes17[];
+using L for uint;
+using L for int;
+using L for bytes18;
+using L for S2;
+using L for E2;
+using L for mapping(int => uint128);
+using L for mapping(int128 => uint);
+using L for string[][];
+using L for bytes[][];
+using L for custom_int;

tests/e2e/detectors/test_detectors.py

@@ -1639,6 +1639,11 @@ def id_test(test_item: Test):
         "LowCyclomaticComplexity.sol",
         "0.8.16",
     ),
+    Test(
+        all_detectors.IncorrectUsingFor,
+        "IncorrectUsingForTopLevel.sol",
+        "0.8.17",
+    ),
     Test(
         all_detectors.EncodePackedCollision,
         "encode_packed_collision.sol",