From 247a90edbc94aa512232f3c32e675d8a7e71f265 Mon Sep 17 00:00:00 2001 From: Tigran Avagyan Date: Tue, 13 Feb 2024 19:03:25 +0400 Subject: [PATCH 1/4] fixed immediate inheritance --- slither/solc_parsing/slither_compilation_unit_solc.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/slither/solc_parsing/slither_compilation_unit_solc.py b/slither/solc_parsing/slither_compilation_unit_solc.py index 85921ce74..de8725b94 100644 --- a/slither/solc_parsing/slither_compilation_unit_solc.py +++ b/slither/solc_parsing/slither_compilation_unit_solc.py @@ -456,9 +456,12 @@ Please rename it, this name is reserved for Slither's internals""" # Resolve immediate base contracts for i in contract_parser.baseContracts: if i in contract_parser.remapping: + contract_name = contract_parser.remapping[i] + if contract_name in contract_parser.underlying_contract.file_scope.renaming: + contract_name = contract_parser.underlying_contract.file_scope.renaming[contract_name] fathers.append( contract_parser.underlying_contract.file_scope.get_contract_from_name( - contract_parser.remapping[i] + contract_name ) # self._compilation_unit.get_contract_from_name(contract_parser.remapping[i]) ) From 50f1b98b94f962db6a6b3e6d788b6a1df2834215 Mon Sep 17 00:00:00 2001 From: Tigran Avagyan Date: Tue, 13 Feb 2024 19:16:15 +0400 Subject: [PATCH 2/4] reformatted --- slither/solc_parsing/slither_compilation_unit_solc.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/slither/solc_parsing/slither_compilation_unit_solc.py b/slither/solc_parsing/slither_compilation_unit_solc.py index de8725b94..21dbfff70 100644 --- a/slither/solc_parsing/slither_compilation_unit_solc.py +++ b/slither/solc_parsing/slither_compilation_unit_solc.py @@ -458,7 +458,9 @@ Please rename it, this name is reserved for Slither's internals""" if i in contract_parser.remapping: contract_name = contract_parser.remapping[i] if contract_name in contract_parser.underlying_contract.file_scope.renaming: - contract_name = contract_parser.underlying_contract.file_scope.renaming[contract_name] + contract_name = contract_parser.underlying_contract.file_scope.renaming[ + contract_name + ] fathers.append( contract_parser.underlying_contract.file_scope.get_contract_from_name( contract_name From 3ff3103cfab9d3df59872885186c8e8510a841d0 Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Mon, 19 Feb 2024 21:34:15 -0600 Subject: [PATCH 3/4] fix: support renaming in base inheritance and base constructor calls --- .../slither_compilation_unit_solc.py | 62 ++++++++----------- .../test_data/inheritance_with_renaming/a.sol | 4 ++ .../test_data/inheritance_with_renaming/b.sol | 4 ++ .../test_data/inheritance_with_renaming/c.sol | 3 + tests/unit/core/test_inheritance.py | 36 +++++++++++ 5 files changed, 73 insertions(+), 36 deletions(-) create mode 100644 tests/unit/core/test_data/inheritance_with_renaming/a.sol create mode 100644 tests/unit/core/test_data/inheritance_with_renaming/b.sol create mode 100644 tests/unit/core/test_data/inheritance_with_renaming/c.sol create mode 100644 tests/unit/core/test_inheritance.py diff --git a/slither/solc_parsing/slither_compilation_unit_solc.py b/slither/solc_parsing/slither_compilation_unit_solc.py index 21dbfff70..d67847a53 100644 --- a/slither/solc_parsing/slither_compilation_unit_solc.py +++ b/slither/solc_parsing/slither_compilation_unit_solc.py @@ -421,66 +421,56 @@ Please rename it, this name is reserved for Slither's internals""" self._contracts_by_id[contract.id] = contract self._compilation_unit.contracts.append(contract) + def resolve_remapping_and_renaming(contract_parser: ContractSolc, want: str) -> Contract: + contract_name = contract_parser.remapping[want] + if contract_name in contract_parser.underlying_contract.file_scope.renaming: + contract_name = contract_parser.underlying_contract.file_scope.renaming[ + contract_name + ] + target = contract_parser.underlying_contract.file_scope.get_contract_from_name( + contract_name + ) + if target == contract_parser.underlying_contract: + raise InheritanceResolutionError( + "Could not resolve contract inheritance. This is likely caused by an import renaming that collides with existing names (see https://github.com/crytic/slither/issues/1758)." + f"\n Try changing `contract {target}` ({target.source_mapping}) to a unique name." + ) + assert target, f"Contract {contract_name} not found" + return target + # Update of the inheritance for contract_parser in self._underlying_contract_to_parser.values(): - # remove the first elem in linearizedBaseContracts as it is the contract itself ancestors = [] fathers = [] father_constructors = [] - # try: - # Resolve linearized base contracts. missing_inheritance = None + # Resolve linearized base contracts. + # Remove the first elem in linearizedBaseContracts as it is the contract itself. for i in contract_parser.linearized_base_contracts[1:]: if i in contract_parser.remapping: - contract_name = contract_parser.remapping[i] - if contract_name in contract_parser.underlying_contract.file_scope.renaming: - contract_name = contract_parser.underlying_contract.file_scope.renaming[ - contract_name - ] - target = contract_parser.underlying_contract.file_scope.get_contract_from_name( - contract_name - ) - if target == contract_parser.underlying_contract: - raise InheritanceResolutionError( - "Could not resolve contract inheritance. This is likely caused by an import renaming that collides with existing names (see https://github.com/crytic/slither/issues/1758)." - f"\n Try changing `contract {target}` ({target.source_mapping}) to a unique name." - ) - assert target, f"Contract {contract_name} not found" + target = resolve_remapping_and_renaming(contract_parser, i) ancestors.append(target) elif i in self._contracts_by_id: ancestors.append(self._contracts_by_id[i]) else: missing_inheritance = i - # Resolve immediate base contracts + # Resolve immediate base contracts. for i in contract_parser.baseContracts: if i in contract_parser.remapping: - contract_name = contract_parser.remapping[i] - if contract_name in contract_parser.underlying_contract.file_scope.renaming: - contract_name = contract_parser.underlying_contract.file_scope.renaming[ - contract_name - ] - fathers.append( - contract_parser.underlying_contract.file_scope.get_contract_from_name( - contract_name - ) - # self._compilation_unit.get_contract_from_name(contract_parser.remapping[i]) - ) + target = resolve_remapping_and_renaming(contract_parser, i) + fathers.append(target) elif i in self._contracts_by_id: fathers.append(self._contracts_by_id[i]) else: missing_inheritance = i - # Resolve immediate base constructor calls + # Resolve immediate base constructor calls. for i in contract_parser.baseConstructorContractsCalled: if i in contract_parser.remapping: - father_constructors.append( - contract_parser.underlying_contract.file_scope.get_contract_from_name( - contract_parser.remapping[i] - ) - # self._compilation_unit.get_contract_from_name(contract_parser.remapping[i]) - ) + target = resolve_remapping_and_renaming(contract_parser, i) + father_constructors.append(target) elif i in self._contracts_by_id: father_constructors.append(self._contracts_by_id[i]) else: diff --git a/tests/unit/core/test_data/inheritance_with_renaming/a.sol b/tests/unit/core/test_data/inheritance_with_renaming/a.sol new file mode 100644 index 000000000..6f1429e91 --- /dev/null +++ b/tests/unit/core/test_data/inheritance_with_renaming/a.sol @@ -0,0 +1,4 @@ +import {B as Base} from "./b.sol"; +contract A is Base(address(0)) { + constructor (address x) {} +} \ No newline at end of file diff --git a/tests/unit/core/test_data/inheritance_with_renaming/b.sol b/tests/unit/core/test_data/inheritance_with_renaming/b.sol new file mode 100644 index 000000000..dd85ad3dd --- /dev/null +++ b/tests/unit/core/test_data/inheritance_with_renaming/b.sol @@ -0,0 +1,4 @@ +import {C as C2} from "./c.sol"; +contract B is C2 { + constructor (address x) C2(x) {} +} \ No newline at end of file diff --git a/tests/unit/core/test_data/inheritance_with_renaming/c.sol b/tests/unit/core/test_data/inheritance_with_renaming/c.sol new file mode 100644 index 000000000..722acdd21 --- /dev/null +++ b/tests/unit/core/test_data/inheritance_with_renaming/c.sol @@ -0,0 +1,3 @@ +contract C { + constructor (address) {} +} \ No newline at end of file diff --git a/tests/unit/core/test_inheritance.py b/tests/unit/core/test_inheritance.py new file mode 100644 index 000000000..a34f15ed4 --- /dev/null +++ b/tests/unit/core/test_inheritance.py @@ -0,0 +1,36 @@ +from pathlib import Path + +from slither import Slither +from crytic_compile import CryticCompile +from crytic_compile.platform.solc_standard_json import SolcStandardJson + +TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data" / "inheritance_with_renaming" + +# https://github.com/crytic/slither/issues/2304 +def test_inheritance_with_renaming(solc_binary_path) -> None: + solc_path = solc_binary_path("0.8.15") + standard_json = SolcStandardJson() + for source_file in Path(TEST_DATA_DIR).rglob("*.sol"): + print(source_file) + standard_json.add_source_file(Path(source_file).as_posix()) + compilation = CryticCompile(standard_json, solc=solc_path) + slither = Slither(compilation) + + a = slither.get_contract_from_name("A")[0] + b = slither.get_contract_from_name("B")[0] + c = slither.get_contract_from_name("C")[0] + + assert len(a.immediate_inheritance) == 1 + assert a.immediate_inheritance[0] == b + assert len(a.inheritance) == 2 + assert a.inheritance[0] == b + assert a.inheritance[1] == c + assert len(a.explicit_base_constructor_calls) == 1 + a_base_constructor_call = a.explicit_base_constructor_calls[0] + assert a_base_constructor_call == b.constructor + + assert len(b.inheritance) == 1 + assert b.inheritance[0] == c + assert len(b.immediate_inheritance) == 1 + assert b.immediate_inheritance[0] == c + assert len(b.explicit_base_constructor_calls) == 0 From e580b2ff2890c24f13ce121df37f6ffa9d7ee80b Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Mon, 19 Feb 2024 21:39:08 -0600 Subject: [PATCH 4/4] pylint --- tests/unit/core/test_inheritance.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/core/test_inheritance.py b/tests/unit/core/test_inheritance.py index a34f15ed4..d3702e2a5 100644 --- a/tests/unit/core/test_inheritance.py +++ b/tests/unit/core/test_inheritance.py @@ -1,8 +1,8 @@ from pathlib import Path -from slither import Slither from crytic_compile import CryticCompile from crytic_compile.platform.solc_standard_json import SolcStandardJson +from slither import Slither TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data" / "inheritance_with_renaming"