From 14f4a32feb31e93e61f27366840e97be75234507 Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Thu, 21 Mar 2024 11:20:18 -0500 Subject: [PATCH 1/7] register references for HighLevelCall's --- slither/slithir/convert.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/slither/slithir/convert.py b/slither/slithir/convert.py index 170df8cba9..ec8f6a081f 100644 --- a/slither/slithir/convert.py +++ b/slither/slithir/convert.py @@ -2014,6 +2014,9 @@ def _find_source_mapping_references(irs: List[Operation]) -> None: if isinstance(ir, NewContract): ir.contract_created.references.append(ir.expression.source_mapping) + if isinstance(ir, HighLevelCall): + ir.function.references.append(ir.expression.source_mapping) + # endregion ################################################################################### From 28688b334be9966807e90d2c9db33d28cdb3d88d Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Thu, 21 Mar 2024 12:39:25 -0500 Subject: [PATCH 2/7] fix: add offsets for state variables --- slither/core/slither_core.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/slither/core/slither_core.py b/slither/core/slither_core.py index 76220f5bae..f091880cdc 100644 --- a/slither/core/slither_core.py +++ b/slither/core/slither_core.py @@ -260,6 +260,9 @@ def _compute_offsets_to_ref_impl_decl(self): # pylint: disable=too-many-branche for variable in modifier.local_variables: self._compute_offsets_from_thing(variable) + for var in contract.state_variables: + self._compute_offsets_from_thing(var) + for st in contract.structures: self._compute_offsets_from_thing(st) From fc9416c3ecd1f6d2cb32770d6ec473493ff3db87 Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Thu, 21 Mar 2024 22:16:01 -0500 Subject: [PATCH 3/7] fix: attach references for inheritance specifier and top level types --- slither/core/slither_core.py | 12 ++++ slither/solc_parsing/declarations/contract.py | 14 ++-- .../slither_compilation_unit_solc.py | 15 ++-- slither/utils/source_mapping.py | 17 ++++- .../test_data/src_mapping/inheritance.sol | 18 +++++ tests/unit/core/test_source_mapping.py | 69 ++++++++++++++++++- 6 files changed, 131 insertions(+), 14 deletions(-) diff --git a/slither/core/slither_core.py b/slither/core/slither_core.py index f091880cdc..2dc0cbd8c1 100644 --- a/slither/core/slither_core.py +++ b/slither/core/slither_core.py @@ -271,6 +271,10 @@ def _compute_offsets_to_ref_impl_decl(self): # pylint: disable=too-many-branche for event in contract.events: self._compute_offsets_from_thing(event) + + for typ in contract.type_aliases: + self._compute_offsets_from_thing(typ) + for enum in compilation_unit.enums_top_level: self._compute_offsets_from_thing(enum) for event in compilation_unit.events_top_level: @@ -279,6 +283,14 @@ def _compute_offsets_to_ref_impl_decl(self): # pylint: disable=too-many-branche self._compute_offsets_from_thing(function) for st in compilation_unit.structures_top_level: self._compute_offsets_from_thing(st) + for var in compilation_unit.variables_top_level: + self._compute_offsets_from_thing(var) + for typ in compilation_unit.type_aliases.values(): + self._compute_offsets_from_thing(typ) + for err in compilation_unit.custom_errors: + self._compute_offsets_from_thing(err) + for event in compilation_unit.events_top_level: + self._compute_offsets_from_thing(event) for import_directive in compilation_unit.import_directives: self._compute_offsets_from_thing(import_directive) for pragma in compilation_unit.pragma_directives: diff --git a/slither/solc_parsing/declarations/contract.py b/slither/solc_parsing/declarations/contract.py index f0696d557b..4dd975474a 100644 --- a/slither/solc_parsing/declarations/contract.py +++ b/slither/solc_parsing/declarations/contract.py @@ -1,6 +1,6 @@ import logging import re -from typing import Any, List, Dict, Callable, TYPE_CHECKING, Union, Set, Sequence +from typing import Any, List, Dict, Callable, TYPE_CHECKING, Union, Set, Sequence, Tuple from slither.core.declarations import ( Modifier, @@ -64,7 +64,8 @@ def __init__( # use to remap inheritance id self._remapping: Dict[str, str] = {} - self.baseContracts: List[str] = [] + # (referencedDeclaration, offset) + self.baseContracts: List[Tuple[int, str]] = [] self.baseConstructorContractsCalled: List[str] = [] self._linearized_base_contracts: List[int] @@ -201,7 +202,9 @@ def _parse_base_contract_info(self) -> None: # pylint: disable=too-many-branche # Obtain our contract reference and add it to our base contract list referencedDeclaration = base_contract["baseName"]["referencedDeclaration"] - self.baseContracts.append(referencedDeclaration) + self.baseContracts.append( + (referencedDeclaration, base_contract["baseName"]["src"]) + ) # If we have defined arguments in our arguments object, this is a constructor invocation. # (note: 'arguments' can be [], which is not the same as None. [] implies a constructor was @@ -233,7 +236,10 @@ def _parse_base_contract_info(self) -> None: # pylint: disable=too-many-branche referencedDeclaration = base_contract_items[0]["attributes"][ "referencedDeclaration" ] - self.baseContracts.append(referencedDeclaration) + + self.baseContracts.append( + (referencedDeclaration, base_contract_items[0]["src"]) + ) # If we have an 'attributes'->'arguments' which is None, this is not a constructor call. if ( diff --git a/slither/solc_parsing/slither_compilation_unit_solc.py b/slither/solc_parsing/slither_compilation_unit_solc.py index ee28a7bf5d..2c50748425 100644 --- a/slither/solc_parsing/slither_compilation_unit_solc.py +++ b/slither/solc_parsing/slither_compilation_unit_solc.py @@ -479,13 +479,16 @@ def resolve_remapping_and_renaming(contract_parser: ContractSolc, want: str) -> else: missing_inheritance = i - # Resolve immediate base contracts. - for i in contract_parser.baseContracts: - if i in contract_parser.remapping: - target = resolve_remapping_and_renaming(contract_parser, i) + # Resolve immediate base contracts and attach references. + for (id, src) in contract_parser.baseContracts: + if id in contract_parser.remapping: + target = resolve_remapping_and_renaming(contract_parser, id) fathers.append(target) - elif i in self._contracts_by_id: - fathers.append(self._contracts_by_id[i]) + target.add_reference_from_raw_source(src, self.compilation_unit) + elif id in self._contracts_by_id: + target = self._contracts_by_id[id] + fathers.append(target) + target.add_reference_from_raw_source(src, self.compilation_unit) else: missing_inheritance = i diff --git a/slither/utils/source_mapping.py b/slither/utils/source_mapping.py index b117cd5f78..fe3f908b4a 100644 --- a/slither/utils/source_mapping.py +++ b/slither/utils/source_mapping.py @@ -1,7 +1,16 @@ from typing import List from crytic_compile import CryticCompile -from slither.core.declarations import Contract, Function, Enum, Event, Import, Pragma, Structure -from slither.core.solidity_types.type import Type +from slither.core.declarations import ( + Contract, + Function, + Enum, + Event, + Import, + Pragma, + Structure, + CustomError, +) +from slither.core.solidity_types import Type, TypeAlias from slither.core.source_mapping.source_mapping import Source, SourceMapping from slither.core.variables.variable import Variable from slither.exceptions import SlitherError @@ -15,6 +24,10 @@ def get_definition(target: SourceMapping, crytic_compile: CryticCompile) -> Sour pattern = "import" elif isinstance(target, Pragma): pattern = "pragma" # todo maybe return with the while pragma statement + elif isinstance(target, CustomError): + pattern = "error" + elif isinstance(target, TypeAlias): + pattern = "type" elif isinstance(target, Type): raise SlitherError("get_definition_generic not implemented for types") else: diff --git a/tests/unit/core/test_data/src_mapping/inheritance.sol b/tests/unit/core/test_data/src_mapping/inheritance.sol index 4c3ff92ee7..aa7138dba3 100644 --- a/tests/unit/core/test_data/src_mapping/inheritance.sol +++ b/tests/unit/core/test_data/src_mapping/inheritance.sol @@ -33,3 +33,21 @@ contract C is A{ contract D is A{ } + + +type T is uint256; +uint constant U = 1; +error V(T); +event W(T); + +contract E { + type X is int256; + function f() public { + T t = T.wrap(U); + if (T.unwrap(t) == 0) { + revert V(t); + } + emit W(t); + X x = X.wrap(1); + } +} diff --git a/tests/unit/core/test_source_mapping.py b/tests/unit/core/test_source_mapping.py index 9577014297..c5e13a0d3b 100644 --- a/tests/unit/core/test_source_mapping.py +++ b/tests/unit/core/test_source_mapping.py @@ -1,17 +1,26 @@ from pathlib import Path from slither import Slither -from slither.core.declarations import Function +from slither.core.declarations import Function, CustomErrorTopLevel, EventTopLevel +from slither.core.solidity_types.type_alias import TypeAliasTopLevel, TypeAliasContract +from slither.core.variables.top_level_variable import TopLevelVariable TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data" SRC_MAPPING_TEST_ROOT = Path(TEST_DATA_DIR, "src_mapping") -def test_source_mapping(solc_binary_path): +def test_source_mapping_inheritance(solc_binary_path): solc_path = solc_binary_path("0.6.12") file = Path(SRC_MAPPING_TEST_ROOT, "inheritance.sol").as_posix() slither = Slither(file, solc=solc_path) + # 3 reference to A in inheritance `contract $ is A` + assert {(x.start, x.end) for x in slither.offset_to_references(file, 9)} == { + (121, 122), + (185, 186), + (299, 300), + } + # Check if A.f() is at the offset 27 functions = slither.offset_to_objects(file, 27) assert len(functions) == 1 @@ -113,6 +122,62 @@ def test_references_user_defined_types_when_casting(solc_binary_path): assert lines == [12, 18] +def test_source_mapping_inheritance(solc_binary_path): + solc_path = solc_binary_path("0.8.24") + file = Path(SRC_MAPPING_TEST_ROOT, "inheritance.sol").as_posix() + slither = Slither(file, solc=solc_path) + + # Check if T is at the offset 312 + types = slither.offset_to_objects(file, 312) + assert len(types) == 1 + type_ = types.pop() + assert isinstance(type_, TypeAliasTopLevel) + assert type_.name == "T" + + assert {(x.start, x.end) for x in slither.offset_to_references(file, 312)} == { + (355, 356), + (367, 368), + (441, 442), + (447, 448), + (470, 471), + } + + # Check if U is at the offset 340 + constants = slither.offset_to_objects(file, 340) + assert len(constants) == 1 + constant = constants.pop() + assert isinstance(constant, TopLevelVariable) + assert constant.name == "U" + assert {(x.start, x.end) for x in slither.offset_to_references(file, 340)} == {(454, 455)} + + # Check if V is at the offset 353 + errors = slither.offset_to_objects(file, 353) + assert len(errors) == 1 + error = errors.pop() + assert isinstance(error, CustomErrorTopLevel) + assert error.name == "V" + assert {(x.start, x.end) for x in slither.offset_to_references(file, 353)} == {(509, 510)} + + # Check if W is at the offset 365 + events = slither.offset_to_objects(file, 365) + assert len(events) == 1 + event = events.pop() + assert isinstance(event, EventTopLevel) + assert event.name == "W" + assert {(x.start, x.end) for x in slither.offset_to_references(file, 365)} == {(538, 539)} + + # Check if X is at the offset 394 + types = slither.offset_to_objects(file, 394) + assert len(types) == 1 + type_ = types.pop() + assert isinstance(type_, TypeAliasContract) + assert type_.name == "X" + assert {(x.start, x.end) for x in slither.offset_to_references(file, 394)} == { + (552, 553), + (558, 559), + } + + def test_references_self_identifier(): """ Tests that shadowing state variables with local variables does not affect references. From db6ff6618f891087f4f735a91c7b6efcea79bc64 Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Fri, 22 Mar 2024 11:17:17 -0500 Subject: [PATCH 4/7] fix pylint --- slither/slithir/convert.py | 2 +- slither/solc_parsing/slither_compilation_unit_solc.py | 10 +++++----- tests/unit/core/test_source_mapping.py | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/slither/slithir/convert.py b/slither/slithir/convert.py index ec8f6a081f..e2b1fe7ba6 100644 --- a/slither/slithir/convert.py +++ b/slither/slithir/convert.py @@ -1209,7 +1209,7 @@ def extract_tmp_call(ins: TmpCall, contract: Optional[Contract]) -> Union[Call, internalcall.set_expression(ins.expression) return internalcall - raise Exception(f"Not extracted {type(ins.called)} {ins}") + raise SlithIRError(f"Not extracted {type(ins.called)} {ins}") # endregion diff --git a/slither/solc_parsing/slither_compilation_unit_solc.py b/slither/solc_parsing/slither_compilation_unit_solc.py index 2c50748425..7dc8cc6e52 100644 --- a/slither/solc_parsing/slither_compilation_unit_solc.py +++ b/slither/solc_parsing/slither_compilation_unit_solc.py @@ -480,13 +480,13 @@ def resolve_remapping_and_renaming(contract_parser: ContractSolc, want: str) -> missing_inheritance = i # Resolve immediate base contracts and attach references. - for (id, src) in contract_parser.baseContracts: - if id in contract_parser.remapping: - target = resolve_remapping_and_renaming(contract_parser, id) + for (i, src) in contract_parser.baseContracts: + if i in contract_parser.remapping: + target = resolve_remapping_and_renaming(contract_parser, i) fathers.append(target) target.add_reference_from_raw_source(src, self.compilation_unit) - elif id in self._contracts_by_id: - target = self._contracts_by_id[id] + elif i in self._contracts_by_id: + target = self._contracts_by_id[i] fathers.append(target) target.add_reference_from_raw_source(src, self.compilation_unit) else: diff --git a/tests/unit/core/test_source_mapping.py b/tests/unit/core/test_source_mapping.py index c5e13a0d3b..3042521a79 100644 --- a/tests/unit/core/test_source_mapping.py +++ b/tests/unit/core/test_source_mapping.py @@ -122,7 +122,7 @@ def test_references_user_defined_types_when_casting(solc_binary_path): assert lines == [12, 18] -def test_source_mapping_inheritance(solc_binary_path): +def test_source_mapping_top_level_defs(solc_binary_path): solc_path = solc_binary_path("0.8.24") file = Path(SRC_MAPPING_TEST_ROOT, "inheritance.sol").as_posix() slither = Slither(file, solc=solc_path) From 209df4ea76d131365c46d6556f767cb45185ba4e Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Fri, 22 Mar 2024 11:28:45 -0500 Subject: [PATCH 5/7] split test into its own file --- .../src_mapping/TopLevelReferences.sol | 16 +++++++ .../test_data/src_mapping/inheritance.sol | 18 -------- tests/unit/core/test_source_mapping.py | 46 +++++++++---------- 3 files changed, 39 insertions(+), 41 deletions(-) create mode 100644 tests/unit/core/test_data/src_mapping/TopLevelReferences.sol diff --git a/tests/unit/core/test_data/src_mapping/TopLevelReferences.sol b/tests/unit/core/test_data/src_mapping/TopLevelReferences.sol new file mode 100644 index 0000000000..68f7b48ad9 --- /dev/null +++ b/tests/unit/core/test_data/src_mapping/TopLevelReferences.sol @@ -0,0 +1,16 @@ +type T is uint256; +uint constant U = 1; +error V(T); +event W(T); + +contract E { + type X is int256; + function f() public { + T t = T.wrap(U); + if (T.unwrap(t) == 0) { + revert V(t); + } + emit W(t); + X x = X.wrap(1); + } +} \ No newline at end of file diff --git a/tests/unit/core/test_data/src_mapping/inheritance.sol b/tests/unit/core/test_data/src_mapping/inheritance.sol index aa7138dba3..4c3ff92ee7 100644 --- a/tests/unit/core/test_data/src_mapping/inheritance.sol +++ b/tests/unit/core/test_data/src_mapping/inheritance.sol @@ -33,21 +33,3 @@ contract C is A{ contract D is A{ } - - -type T is uint256; -uint constant U = 1; -error V(T); -event W(T); - -contract E { - type X is int256; - function f() public { - T t = T.wrap(U); - if (T.unwrap(t) == 0) { - revert V(t); - } - emit W(t); - X x = X.wrap(1); - } -} diff --git a/tests/unit/core/test_source_mapping.py b/tests/unit/core/test_source_mapping.py index 3042521a79..c83624d578 100644 --- a/tests/unit/core/test_source_mapping.py +++ b/tests/unit/core/test_source_mapping.py @@ -124,57 +124,57 @@ def test_references_user_defined_types_when_casting(solc_binary_path): def test_source_mapping_top_level_defs(solc_binary_path): solc_path = solc_binary_path("0.8.24") - file = Path(SRC_MAPPING_TEST_ROOT, "inheritance.sol").as_posix() + file = Path(SRC_MAPPING_TEST_ROOT, "TopLevelReferences.sol").as_posix() slither = Slither(file, solc=solc_path) - # Check if T is at the offset 312 - types = slither.offset_to_objects(file, 312) + # Check if T is at the offset 5 + types = slither.offset_to_objects(file, 5) assert len(types) == 1 type_ = types.pop() assert isinstance(type_, TypeAliasTopLevel) assert type_.name == "T" - assert {(x.start, x.end) for x in slither.offset_to_references(file, 312)} == { - (355, 356), - (367, 368), - (441, 442), - (447, 448), - (470, 471), + assert {(x.start, x.end) for x in slither.offset_to_references(file, 5)} == { + (48, 49), + (60, 61), + (134, 135), + (140, 141), + (163, 164), } - # Check if U is at the offset 340 - constants = slither.offset_to_objects(file, 340) + # Check if U is at the offset 33 + constants = slither.offset_to_objects(file, 33) assert len(constants) == 1 constant = constants.pop() assert isinstance(constant, TopLevelVariable) assert constant.name == "U" - assert {(x.start, x.end) for x in slither.offset_to_references(file, 340)} == {(454, 455)} + assert {(x.start, x.end) for x in slither.offset_to_references(file, 33)} == {(147, 148)} - # Check if V is at the offset 353 - errors = slither.offset_to_objects(file, 353) + # Check if V is at the offset 46 + errors = slither.offset_to_objects(file, 46) assert len(errors) == 1 error = errors.pop() assert isinstance(error, CustomErrorTopLevel) assert error.name == "V" - assert {(x.start, x.end) for x in slither.offset_to_references(file, 353)} == {(509, 510)} + assert {(x.start, x.end) for x in slither.offset_to_references(file, 46)} == {(202, 203)} - # Check if W is at the offset 365 - events = slither.offset_to_objects(file, 365) + # Check if W is at the offset 58 + events = slither.offset_to_objects(file, 58) assert len(events) == 1 event = events.pop() assert isinstance(event, EventTopLevel) assert event.name == "W" - assert {(x.start, x.end) for x in slither.offset_to_references(file, 365)} == {(538, 539)} + assert {(x.start, x.end) for x in slither.offset_to_references(file, 58)} == {(231, 232)} - # Check if X is at the offset 394 - types = slither.offset_to_objects(file, 394) + # Check if X is at the offset 87 + types = slither.offset_to_objects(file, 87) assert len(types) == 1 type_ = types.pop() assert isinstance(type_, TypeAliasContract) assert type_.name == "X" - assert {(x.start, x.end) for x in slither.offset_to_references(file, 394)} == { - (552, 553), - (558, 559), + assert {(x.start, x.end) for x in slither.offset_to_references(file, 87)} == { + (245, 246), + (251, 252), } From 84c3199458d4cecc4fdcd0b2790e224b84069528 Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Fri, 22 Mar 2024 15:31:18 -0500 Subject: [PATCH 6/7] add regression tests for reference API on windows --- tests/unit/core/test_source_mapping.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/unit/core/test_source_mapping.py b/tests/unit/core/test_source_mapping.py index c83624d578..80a7985edc 100644 --- a/tests/unit/core/test_source_mapping.py +++ b/tests/unit/core/test_source_mapping.py @@ -1,5 +1,5 @@ from pathlib import Path - +import pytest from slither import Slither from slither.core.declarations import Function, CustomErrorTopLevel, EventTopLevel from slither.core.solidity_types.type_alias import TypeAliasTopLevel, TypeAliasContract @@ -8,9 +8,10 @@ TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data" SRC_MAPPING_TEST_ROOT = Path(TEST_DATA_DIR, "src_mapping") - -def test_source_mapping_inheritance(solc_binary_path): - solc_path = solc_binary_path("0.6.12") +# Ensure issue fixed in https://github.com/crytic/crytic-compile/pull/554 does not regress in Slither's reference lookup. +@pytest.mark.parametrize("solc_version", ["0.6.12", "0.8.7", "0.8.8"]) +def test_source_mapping_inheritance(solc_binary_path, solc_version): + solc_path = solc_binary_path(solc_version) file = Path(SRC_MAPPING_TEST_ROOT, "inheritance.sol").as_posix() slither = Slither(file, solc=solc_path) From 93bbb22f4abeb95e951fae39849a761bf0050f9e Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Fri, 22 Mar 2024 15:45:36 -0500 Subject: [PATCH 7/7] use crytic-compile master with windows path fix --- setup.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/setup.py b/setup.py index 857b371605..42f6af1522 100644 --- a/setup.py +++ b/setup.py @@ -15,8 +15,8 @@ "packaging", "prettytable>=3.3.0", "pycryptodome>=3.4.6", - "crytic-compile>=0.3.5,<0.4.0", - # "crytic-compile@git+https://github.com/crytic/crytic-compile.git@master#egg=crytic-compile", + # "crytic-compile>=0.3.5,<0.4.0", + "crytic-compile@git+https://github.com/crytic/crytic-compile.git@master#egg=crytic-compile", "web3>=6.0.0", "eth-abi>=4.0.0", "eth-typing>=3.0.0",