Merge branch 'dev' into dev-revisit-calls-api

.github/actions/upload-coverage/action.yml

@@ -27,4 +27,5 @@ runs:
         path: |
           .coverage.*
           *.lcov
-        if-no-files-found: ignore
+        if-no-files-found: ignore
+        include-hidden-files: true

.pre-commit-hooks.yaml

@@ -0,0 +1,9 @@
+- id: slither
+  name: Slither
+  description: Run Slither on your project
+  entry: slither
+  args:
+    - .
+  pass_filenames: false
+  language: python
+  files: \.sol$

FUNDING.json

@@ -3,5 +3,10 @@
         "op-mainnet": {
             "ownedBy": "0xc44F30Be3eBBEfdDBB5a85168710b4f0e18f4Ff0"
         }
+    },
+    "drips": {
+        "ethereum": {
+            "ownedBy": "0x5e2BA02F62bD4efa939e3B80955bBC21d015DbA0"
+        }
     }
 }

README.md

@@ -102,6 +102,13 @@ docker run -it -v /home/share:/share trailofbits/eth-security-toolbox
 ### Integration
 
 * For GitHub action integration, use [slither-action](https://github.com/marketplace/actions/slither-action).
+* For pre-commit integration, use (replace `$GIT_TAG` with real tag)
+  ```YAML
+  - repo: https://github.com/crytic/slither
+    rev: $GIT_TAG
+    hooks:
+      - id: slither
+  ```
 * To generate a Markdown report, use `slither [target] --checklist`.
 * To generate a Markdown with GitHub source code highlighting, use `slither [target] --checklist --markdown-root https://github.com/ORG/REPO/blob/COMMIT/` (replace `ORG`, `REPO`, `COMMIT`)
 

setup.py

@@ -8,7 +8,7 @@ setup(
     description="Slither is a Solidity and Vyper static analysis framework written in Python 3.",
     url="https://github.com/crytic/slither",
     author="Trail of Bits",
-    version="0.10.3",
+    version="0.10.4",
     packages=find_packages(),
     python_requires=">=3.8",
     install_requires=[

slither/core/cfg/node.py

@@ -518,7 +518,7 @@ class Node(SourceMapping):  # pylint: disable=too-many-public-methods
             bool: True if the node has a require or assert call
         """
         return any(
-            ir.function.name in ["require(bool)", "require(bool,string)", "assert(bool)"]
+            ir.function.name in ["require(bool)", "require(bool,string)", "require(bool,error)", "assert(bool)"]
             for ir in self.internal_calls
         )
 

slither/core/declarations/solidity_variables.py

@@ -50,6 +50,7 @@ SOLIDITY_FUNCTIONS: Dict[str, List[str]] = {
     "assert(bool)": [],
     "require(bool)": [],
     "require(bool,string)": [],
+    "require(bool,error)": [],  # Solidity 0.8.26 via-ir and Solidity >= 0.8.27
     "revert()": [],
     "revert(string)": [],
     "revert ": [],

slither/core/slither_core.py

@@ -8,7 +8,7 @@ import pathlib
 import posixpath
 import re
 from collections import defaultdict
-from typing import Optional, Dict, List, Set, Union, Tuple
+from typing import Optional, Dict, List, Set, Union, Tuple, TypeVar
 
 from crytic_compile import CryticCompile
 from crytic_compile.utils.naming import Filename
@@ -88,6 +88,7 @@ class SlitherCore(Context):
         self._contracts: List[Contract] = []
         self._contracts_derived: List[Contract] = []
 
+        self._offset_to_min_offset: Optional[Dict[Filename, Dict[int, Set[int]]]] = None
         self._offset_to_objects: Optional[Dict[Filename, Dict[int, Set[SourceMapping]]]] = None
         self._offset_to_references: Optional[Dict[Filename, Dict[int, Set[Source]]]] = None
         self._offset_to_implementations: Optional[Dict[Filename, Dict[int, Set[Source]]]] = None
@@ -195,69 +196,70 @@ class SlitherCore(Context):
                 for f in c.functions:
                     f.cfg_to_dot(os.path.join(d, f"{c.name}.{f.name}.dot"))
 
-    def offset_to_objects(self, filename_str: str, offset: int) -> Set[SourceMapping]:
-        if self._offset_to_objects is None:
-            self._compute_offsets_to_ref_impl_decl()
-        filename: Filename = self.crytic_compile.filename_lookup(filename_str)
-        return self._offset_to_objects[filename][offset]
-
     def _compute_offsets_from_thing(self, thing: SourceMapping):
         definition = get_definition(thing, self.crytic_compile)
         references = get_references(thing)
         implementations = get_all_implementations(thing, self.contracts)
 
+        # Create the offset mapping
         for offset in range(definition.start, definition.end + 1):
-            if (
-                isinstance(thing, (TopLevel, Contract))
-                or (
-                    isinstance(thing, FunctionContract)
-                    and thing.contract_declarer == thing.contract
-                )
-                or (isinstance(thing, ContractLevel) and not isinstance(thing, FunctionContract))
-            ):
+            self._offset_to_min_offset[definition.filename][offset].add(definition.start)
 
-                self._offset_to_objects[definition.filename][offset].add(thing)
+        is_declared_function = (
+            isinstance(thing, FunctionContract) and thing.contract_declarer == thing.contract
+        )
 
-            self._offset_to_definitions[definition.filename][offset].add(definition)
-            self._offset_to_implementations[definition.filename][offset].update(implementations)
-            self._offset_to_references[definition.filename][offset] |= set(references)
+        should_add_to_objects = (
+            isinstance(thing, (TopLevel, Contract))
+            or is_declared_function
+            or (isinstance(thing, ContractLevel) and not isinstance(thing, FunctionContract))
+        )
+
+        if should_add_to_objects:
+            self._offset_to_objects[definition.filename][definition.start].add(thing)
+
+        self._offset_to_definitions[definition.filename][definition.start].add(definition)
+        self._offset_to_implementations[definition.filename][definition.start].update(
+            implementations
+        )
+        self._offset_to_references[definition.filename][definition.start] |= set(references)
+
+        # For references
+        should_add_to_objects = (
+            isinstance(thing, TopLevel)
+            or is_declared_function
+            or (isinstance(thing, ContractLevel) and not isinstance(thing, FunctionContract))
+        )
 
         for ref in references:
             for offset in range(ref.start, ref.end + 1):
-                is_declared_function = (
-                    isinstance(thing, FunctionContract)
-                    and thing.contract_declarer == thing.contract
-                )
+                self._offset_to_min_offset[definition.filename][offset].add(ref.start)
+
+            if should_add_to_objects:
+                self._offset_to_objects[definition.filename][ref.start].add(thing)
+
+            if is_declared_function:
+                # Only show the nearest lexical definition for declared contract-level functions
                 if (
-                    isinstance(thing, TopLevel)
-                    or is_declared_function
-                    or (
-                        isinstance(thing, ContractLevel) and not isinstance(thing, FunctionContract)
-                    )
+                    thing.contract.source_mapping.start
+                    < ref.start
+                    < thing.contract.source_mapping.end
                 ):
-                    self._offset_to_objects[definition.filename][offset].add(thing)
-
-                if is_declared_function:
-                    # Only show the nearest lexical definition for declared contract-level functions
-                    if (
-                        thing.contract.source_mapping.start
-                        < offset
-                        < thing.contract.source_mapping.end
-                    ):
 
-                        self._offset_to_definitions[ref.filename][offset].add(definition)
+                    self._offset_to_definitions[ref.filename][ref.start].add(definition)
 
-                else:
-                    self._offset_to_definitions[ref.filename][offset].add(definition)
+            else:
+                self._offset_to_definitions[ref.filename][ref.start].add(definition)
 
-                self._offset_to_implementations[ref.filename][offset].update(implementations)
-                self._offset_to_references[ref.filename][offset] |= set(references)
+            self._offset_to_implementations[ref.filename][ref.start].update(implementations)
+            self._offset_to_references[ref.filename][ref.start] |= set(references)
 
     def _compute_offsets_to_ref_impl_decl(self):  # pylint: disable=too-many-branches
         self._offset_to_references = defaultdict(lambda: defaultdict(lambda: set()))
         self._offset_to_definitions = defaultdict(lambda: defaultdict(lambda: set()))
         self._offset_to_implementations = defaultdict(lambda: defaultdict(lambda: set()))
         self._offset_to_objects = defaultdict(lambda: defaultdict(lambda: set()))
+        self._offset_to_min_offset = defaultdict(lambda: defaultdict(lambda: set()))
 
         for compilation_unit in self._compilation_units:
             for contract in compilation_unit.contracts:
@@ -308,23 +310,59 @@ class SlitherCore(Context):
             for pragma in compilation_unit.pragma_directives:
                 self._compute_offsets_from_thing(pragma)
 
+    T = TypeVar("T", Source, SourceMapping)
+
+    def _get_offset(
+        self, mapping: Dict[Filename, Dict[int, Set[T]]], filename_str: str, offset: int
+    ) -> Set[T]:
+        """Get the Source/SourceMapping referenced by the offset.
+
+        For performance reasons, references are only stored once at the lowest offset.
+        It uses the _offset_to_min_offset mapping to retrieve the correct offsets.
+        As multiple definitions can be related to the same offset, we retrieve all of them.
+
+        :param mapping: Mapping to search for (objects. references, ...)
+        :param filename_str: Filename to consider
+        :param offset: Look-up offset
+        :raises IndexError: When the start offset is not found
+        :return: The corresponding set of Source/SourceMapping
+        """
+        filename: Filename = self.crytic_compile.filename_lookup(filename_str)
+
+        start_offsets = self._offset_to_min_offset[filename][offset]
+        if not start_offsets:
+            msg = f"Unable to find reference for offset {offset}"
+            raise IndexError(msg)
+
+        results = set()
+        for start_offset in start_offsets:
+            results |= mapping[filename][start_offset]
+
+        return results
+
     def offset_to_references(self, filename_str: str, offset: int) -> Set[Source]:
         if self._offset_to_references is None:
             self._compute_offsets_to_ref_impl_decl()
-        filename: Filename = self.crytic_compile.filename_lookup(filename_str)
-        return self._offset_to_references[filename][offset]
+
+        return self._get_offset(self._offset_to_references, filename_str, offset)
 
     def offset_to_implementations(self, filename_str: str, offset: int) -> Set[Source]:
         if self._offset_to_implementations is None:
             self._compute_offsets_to_ref_impl_decl()
-        filename: Filename = self.crytic_compile.filename_lookup(filename_str)
-        return self._offset_to_implementations[filename][offset]
+
+        return self._get_offset(self._offset_to_implementations, filename_str, offset)
 
     def offset_to_definitions(self, filename_str: str, offset: int) -> Set[Source]:
         if self._offset_to_definitions is None:
             self._compute_offsets_to_ref_impl_decl()
-        filename: Filename = self.crytic_compile.filename_lookup(filename_str)
-        return self._offset_to_definitions[filename][offset]
+
+        return self._get_offset(self._offset_to_definitions, filename_str, offset)
+
+    def offset_to_objects(self, filename_str: str, offset: int) -> Set[SourceMapping]:
+        if self._offset_to_objects is None:
+            self._compute_offsets_to_ref_impl_decl()
+
+        return self._get_offset(self._offset_to_objects, filename_str, offset)
 
     # endregion
     ###################################################################################

slither/core/source_mapping/source_mapping.py

@@ -112,12 +112,8 @@ class Source:
         try:
             return (
                 self.start == other.start
-                and self.length == other.length
-                and self.filename == other.filename
+                and self.filename.relative == other.filename.relative
                 and self.is_dependency == other.is_dependency
-                and self.lines == other.lines
-                and self.starting_column == other.starting_column
-                and self.ending_column == other.ending_column
                 and self.end == other.end
             )
         except AttributeError:

slither/printers/summary/require_calls.py

@@ -11,6 +11,7 @@ require_or_assert = [
     SolidityFunction("assert(bool)"),
     SolidityFunction("require(bool)"),
     SolidityFunction("require(bool,string)"),
+    SolidityFunction("require(bool,error)"),
 ]
 
 

slither/slithir/operations/assignment.py

@@ -45,10 +45,19 @@ class Assignment(OperationWithLValue):
 
     def __str__(self) -> str:
         lvalue = self.lvalue
+
+        # When rvalues are functions, we want to properly display their return type
+        # Fix: https://github.com/crytic/slither/issues/2266
+        if isinstance(self.rvalue.type, list):
+            rvalue_type = ",".join(f"{rvalue_type}" for rvalue_type in self.rvalue.type)
+        else:
+            rvalue_type = f"{self.rvalue.type}"
+
         assert lvalue
         if lvalue and isinstance(lvalue, ReferenceVariable):
             points = lvalue.points_to
             while isinstance(points, ReferenceVariable):
                 points = points.points_to
-            return f"{lvalue}({lvalue.type}) (->{points}) := {self.rvalue}({self.rvalue.type})"
-        return f"{lvalue}({lvalue.type}) := {self.rvalue}({self.rvalue.type})"
+            return f"{lvalue}({lvalue.type}) (->{points}) := {self.rvalue}({rvalue_type})"
+
+        return f"{lvalue}({lvalue.type}) := {self.rvalue}({rvalue_type})"

slither/slithir/operations/member.py

@@ -1,5 +1,5 @@
 from typing import List, Union
-from slither.core.declarations import Contract, Function
+from slither.core.declarations import Contract, Function, Event
 from slither.core.declarations.custom_error import CustomError
 from slither.core.declarations.enum import Enum
 from slither.core.declarations.solidity_import_placeholder import SolidityImportPlaceHolder
@@ -33,14 +33,29 @@ class Member(OperationWithLValue):
         # Can be an ElementaryType because of bytes.concat, string.concat
         assert is_valid_rvalue(variable_left) or isinstance(
             variable_left,
-            (Contract, Enum, Function, CustomError, SolidityImportPlaceHolder, ElementaryType),
+            (
+                Contract,
+                Enum,
+                Function,
+                Event,
+                CustomError,
+                SolidityImportPlaceHolder,
+                ElementaryType,
+            ),
         )
 
         assert isinstance(variable_right, Constant)
         assert isinstance(result, ReferenceVariable)
         super().__init__()
         self._variable_left: Union[
-            RVALUE, Contract, Enum, Function, CustomError, SolidityImportPlaceHolder, ElementaryType
+            RVALUE,
+            Contract,
+            Enum,
+            Function,
+            Event,
+            CustomError,
+            SolidityImportPlaceHolder,
+            ElementaryType,
         ] = variable_left
         self._variable_right = variable_right
         self._lvalue = result

tests/e2e/printers/test_data/test_printer_slithir/bug-2266.sol

@@ -0,0 +1,13 @@
+pragma solidity ^0.8.0;
+
+contract A {
+    function add(uint256 a, uint256 b) public returns (uint256) {
+        return a + b;
+    }
+}
+
+contract B is A {
+    function assignFunction() public {
+        function(uint256, uint256) returns (uint256) myFunction = super.add;
+    }
+}

tests/e2e/printers/test_printers.py

@@ -7,6 +7,7 @@ from crytic_compile.platform.solc_standard_json import SolcStandardJson
 
 from slither import Slither
 from slither.printers.inheritance.inheritance_graph import PrinterInheritanceGraph
+from slither.printers.summary.slithir import PrinterSlithIR
 
 
 TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data"
@@ -34,8 +35,7 @@ def test_inheritance_printer(solc_binary_path) -> None:
 
     assert counter["B -> A"] == 2
     assert counter["C -> A"] == 1
-
-    # Lets also test the include/exclude interface behavior
+    # Let also test the include/exclude interface behavior
     # Check that the interface is not included
     assert "MyInterfaceX" not in content
 
@@ -46,3 +46,18 @@ def test_inheritance_printer(solc_binary_path) -> None:
 
     # Remove test generated files
     Path("test_printer.dot").unlink(missing_ok=True)
+
+
+def test_slithir_printer(solc_binary_path) -> None:
+    solc_path = solc_binary_path("0.8.0")
+    standard_json = SolcStandardJson()
+    standard_json.add_source_file(
+        Path(TEST_DATA_DIR, "test_printer_slithir", "bug-2266.sol").as_posix()
+    )
+    compilation = CryticCompile(standard_json, solc=solc_path)
+    slither = Slither(compilation)
+
+    printer = PrinterSlithIR(slither, logger=None)
+    output = printer.output("test_printer_slithir.dot")
+
+    assert "slither.core.solidity_types" not in output.data["description"]

tests/e2e/solc_parsing/test_ast_parsing.py

@@ -475,6 +475,7 @@ ALL_TESTS = [
     Test("solidity-0.8.24.sol", ["0.8.24"], solc_args="--evm-version cancun"),
     Test("scope/inherited_function_scope.sol", ["0.8.24"]),
     Test("using_for_global_user_defined_operator_1.sol", ["0.8.24"]),
+    Test("require-error.sol", ["0.8.27"]),
 ]
 # create the output folder if needed
 try:

tests/e2e/solc_parsing/test_data/expected/require-error.sol-0.8.27-compact.json

@@ -0,0 +1,5 @@
+{
+  "TestToken": {
+    "transferWithRequireError(address,uint256)": "digraph{\n0[label=\"Node Type: ENTRY_POINT 0\n\"];\n0->1;\n1[label=\"Node Type: EXPRESSION 1\n\"];\n1->2;\n2[label=\"Node Type: EXPRESSION 2\n\"];\n2->3;\n3[label=\"Node Type: EXPRESSION 3\n\"];\n}\n"
+  }
+}

tests/e2e/solc_parsing/test_data/require-error.sol

@@ -0,0 +1,20 @@
+pragma solidity 0.8.27;
+
+/// Insufficient balance for transfer. Needed `required` but only
+/// `available` available.
+/// @param available balance available.
+/// @param required requested amount to transfer.
+error InsufficientBalance(uint256 available, uint256 required);
+
+contract TestToken {
+    mapping(address => uint) balance;
+    function transferWithRequireError(address to, uint256 amount) public {
+        require(
+            balance[msg.sender] >= amount,
+            InsufficientBalance(balance[msg.sender], amount)
+        );
+        balance[msg.sender] -= amount;
+        balance[to] += amount;
+    }
+    // ...
+}