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] 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