diff --git a/slither/detectors/all_detectors.py b/slither/detectors/all_detectors.py index 1a4121083..f7f1efaa5 100644 --- a/slither/detectors/all_detectors.py +++ b/slither/detectors/all_detectors.py @@ -89,4 +89,5 @@ from .functions.protected_variable import ProtectedVariables 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 diff --git a/slither/detectors/statements/incorrect_using_for.py b/slither/detectors/statements/incorrect_using_for.py new file mode 100644 index 000000000..e2e87e7e0 --- /dev/null +++ b/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 for `). 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 diff --git a/tests/e2e/detectors/snapshots/detectors__detector_IncorrectUsingFor_0_8_17_IncorrectUsingForTopLevel_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_IncorrectUsingFor_0_8_17_IncorrectUsingForTopLevel_sol__0.txt new file mode 100644 index 000000000..518fba20d --- /dev/null +++ b/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). + diff --git a/tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol b/tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol new file mode 100644 index 000000000..5b173e7b5 --- /dev/null +++ b/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; diff --git a/tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol-0.8.17.zip b/tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol-0.8.17.zip new file mode 100644 index 000000000..9afc56d39 Binary files /dev/null and b/tests/e2e/detectors/test_data/incorrect-using-for/0.8.17/IncorrectUsingForTopLevel.sol-0.8.17.zip differ diff --git a/tests/e2e/detectors/test_detectors.py b/tests/e2e/detectors/test_detectors.py index b3947a62a..d003e7ce0 100644 --- a/tests/e2e/detectors/test_detectors.py +++ b/tests/e2e/detectors/test_detectors.py @@ -1639,6 +1639,11 @@ ALL_TEST_OBJECTS = [ "LowCyclomaticComplexity.sol", "0.8.16", ), + Test( + all_detectors.IncorrectUsingFor, + "IncorrectUsingForTopLevel.sol", + "0.8.17", + ), Test( all_detectors.EncodePackedCollision, "encode_packed_collision.sol",