diff --git a/.github/actions/upload-coverage/action.yml b/.github/actions/upload-coverage/action.yml index 02fb82f65..541b93111 100644 --- a/.github/actions/upload-coverage/action.yml +++ b/.github/actions/upload-coverage/action.yml @@ -27,4 +27,5 @@ runs: path: | .coverage.* *.lcov - if-no-files-found: ignore \ No newline at end of file + if-no-files-found: ignore + include-hidden-files: true diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml new file mode 100644 index 000000000..d561ca429 --- /dev/null +++ b/.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$ diff --git a/FUNDING.json b/FUNDING.json index bbda1e452..189cf2f95 100644 --- a/FUNDING.json +++ b/FUNDING.json @@ -3,5 +3,10 @@ "op-mainnet": { "ownedBy": "0xc44F30Be3eBBEfdDBB5a85168710b4f0e18f4Ff0" } + }, + "drips": { + "ethereum": { + "ownedBy": "0x5e2BA02F62bD4efa939e3B80955bBC21d015DbA0" + } } } diff --git a/README.md b/README.md index 515d6a9f7..660f4f8e8 100644 --- a/README.md +++ b/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`) diff --git a/setup.py b/setup.py index e5739804c..ef9d81f20 100644 --- a/setup.py +++ b/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=[ diff --git a/slither/core/cfg/node.py b/slither/core/cfg/node.py index 06868ab82..1d976af45 100644 --- a/slither/core/cfg/node.py +++ b/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 ) diff --git a/slither/core/declarations/solidity_variables.py b/slither/core/declarations/solidity_variables.py index 8094ab7c3..ce8477ab2 100644 --- a/slither/core/declarations/solidity_variables.py +++ b/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 ": [], diff --git a/slither/core/slither_core.py b/slither/core/slither_core.py index 1206e564b..f5e36ace3 100644 --- a/slither/core/slither_core.py +++ b/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 ################################################################################### diff --git a/slither/core/source_mapping/source_mapping.py b/slither/core/source_mapping/source_mapping.py index 41841f1e8..355aa5538 100644 --- a/slither/core/source_mapping/source_mapping.py +++ b/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: diff --git a/slither/printers/summary/require_calls.py b/slither/printers/summary/require_calls.py index 7823de160..ae79e9ed6 100644 --- a/slither/printers/summary/require_calls.py +++ b/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)"), ] diff --git a/slither/slithir/operations/assignment.py b/slither/slithir/operations/assignment.py index 1f29ceb7b..ab6637faa 100644 --- a/slither/slithir/operations/assignment.py +++ b/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})" diff --git a/slither/slithir/operations/member.py b/slither/slithir/operations/member.py index 0942813cf..55979572c 100644 --- a/slither/slithir/operations/member.py +++ b/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 diff --git a/tests/e2e/printers/test_data/test_printer_slithir/bug-2266.sol b/tests/e2e/printers/test_data/test_printer_slithir/bug-2266.sol new file mode 100644 index 000000000..5c11a2914 --- /dev/null +++ b/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; + } +} \ No newline at end of file diff --git a/tests/e2e/printers/test_printers.py b/tests/e2e/printers/test_printers.py index 3dea8b74a..aa5d7f8a4 100644 --- a/tests/e2e/printers/test_printers.py +++ b/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"] diff --git a/tests/e2e/solc_parsing/test_ast_parsing.py b/tests/e2e/solc_parsing/test_ast_parsing.py index ca3872f8c..6ec7b6fbd 100644 --- a/tests/e2e/solc_parsing/test_ast_parsing.py +++ b/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: diff --git a/tests/e2e/solc_parsing/test_data/compile/require-error.sol-0.8.27-compact.zip b/tests/e2e/solc_parsing/test_data/compile/require-error.sol-0.8.27-compact.zip new file mode 100644 index 000000000..63aa223b3 Binary files /dev/null and b/tests/e2e/solc_parsing/test_data/compile/require-error.sol-0.8.27-compact.zip differ diff --git a/tests/e2e/solc_parsing/test_data/expected/require-error.sol-0.8.27-compact.json b/tests/e2e/solc_parsing/test_data/expected/require-error.sol-0.8.27-compact.json new file mode 100644 index 000000000..3c3089c04 --- /dev/null +++ b/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" + } +} \ No newline at end of file diff --git a/tests/e2e/solc_parsing/test_data/require-error.sol b/tests/e2e/solc_parsing/test_data/require-error.sol new file mode 100644 index 000000000..97c8ecac4 --- /dev/null +++ b/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; + } + // ... +}