From 4ed2a6adf9b57482b2b9a47f2c89ac68869647a5 Mon Sep 17 00:00:00 2001 From: Feist Josselin Date: Tue, 10 Jan 2023 13:11:57 +0100 Subject: [PATCH 1/2] Improve lookup for state variables --- tests/lookup/private_variable.sol | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 tests/lookup/private_variable.sol diff --git a/tests/lookup/private_variable.sol b/tests/lookup/private_variable.sol new file mode 100644 index 0000000000..e69de29bb2 From 33c1fd2c8fe9d62f93047362ec350356a4ca1a0c Mon Sep 17 00:00:00 2001 From: Feist Josselin Date: Tue, 10 Jan 2023 13:12:34 +0100 Subject: [PATCH 2/2] Add files --- slither/core/declarations/contract.py | 9 ++++++++- slither/slithir/convert.py | 2 -- slither/solc_parsing/declarations/contract.py | 12 +++++++++--- .../solc_parsing/slither_compilation_unit_solc.py | 2 +- tests/lookup/private_variable.sol | 13 +++++++++++++ tests/test_features.py | 11 +++++++++++ 6 files changed, 42 insertions(+), 7 deletions(-) diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py index e0191ac2ef..d1feebb056 100644 --- a/slither/core/declarations/contract.py +++ b/slither/core/declarations/contract.py @@ -70,6 +70,8 @@ def __init__(self, compilation_unit: "SlitherCompilationUnit", scope: "FileScope self._enums: Dict[str, "EnumContract"] = {} self._structures: Dict[str, "StructureContract"] = {} self._events: Dict[str, "Event"] = {} + # map accessible variable from name -> variable + # do not contain private variables inherited from contract self._variables: Dict[str, "StateVariable"] = {} self._variables_ordered: List["StateVariable"] = [] self._modifiers: Dict[str, "Modifier"] = {} @@ -330,7 +332,9 @@ def custom_errors_as_dict(self) -> Dict[str, "CustomErrorContract"]: @property def variables(self) -> List["StateVariable"]: """ - list(StateVariable): List of the state variables. Alias to self.state_variables + Returns all the accessible variables (do not include private variable from inherited contract) + + list(StateVariable): List of the state variables. Alias to self.state_variables. """ return list(self.state_variables) @@ -341,6 +345,9 @@ def variables_as_dict(self) -> Dict[str, "StateVariable"]: @property def state_variables(self) -> List["StateVariable"]: """ + Returns all the accessible variables (do not include private variable from inherited contract). + Use state_variables_ordered for all the variables following the storage order + list(StateVariable): List of the state variables. """ return list(self._variables.values()) diff --git a/slither/slithir/convert.py b/slither/slithir/convert.py index 0d2ef1b741..18aa122737 100644 --- a/slither/slithir/convert.py +++ b/slither/slithir/convert.py @@ -1546,7 +1546,6 @@ def convert_type_of_high_and_internal_level_call(ir: Operation, contract: Option else: assert isinstance(ir, HighLevelCall) assert contract - candidates = [ f for f in contract.functions @@ -1554,7 +1553,6 @@ def convert_type_of_high_and_internal_level_call(ir: Operation, contract: Option and not f.is_shadowed and len(f.parameters) == len(ir.arguments) ] - if len(candidates) == 1: func = candidates[0] if func is None: diff --git a/slither/solc_parsing/declarations/contract.py b/slither/solc_parsing/declarations/contract.py index a939144494..0ffd241312 100644 --- a/slither/solc_parsing/declarations/contract.py +++ b/slither/solc_parsing/declarations/contract.py @@ -326,7 +326,13 @@ def parse_custom_errors(self): def parse_state_variables(self): for father in self._contract.inheritance_reverse: - self._contract.variables_as_dict.update(father.variables_as_dict) + self._contract.variables_as_dict.update( + { + name: v + for name, v in father.variables_as_dict.items() + if v.visibility != "private" + } + ) self._contract.add_variables_ordered( [ var @@ -391,11 +397,11 @@ def parse_functions(self): ################################################################################### ################################################################################### - def log_incorrect_parsing(self, error): + def log_incorrect_parsing(self, error: str) -> None: if self._contract.compilation_unit.core.disallow_partial: raise ParsingError(error) LOGGER.error(error) - self._contract.is_incorrectly_parsed = True + self._contract.is_incorrectly_constructed = True def analyze_content_modifiers(self): try: diff --git a/slither/solc_parsing/slither_compilation_unit_solc.py b/slither/solc_parsing/slither_compilation_unit_solc.py index 296ac82381..4f6f9ac5c8 100644 --- a/slither/solc_parsing/slither_compilation_unit_solc.py +++ b/slither/solc_parsing/slither_compilation_unit_solc.py @@ -743,7 +743,7 @@ def _convert_to_slithir(self): # And the interface is redefined due to contract's name reuse # But the available version misses some functions self._underlying_contract_to_parser[contract].log_incorrect_parsing( - f"Impossible to generate IR for {contract.name}.{func.name}:\n {e}" + f"Impossible to generate IR for {contract.name}.{func.name} ({func.source_mapping}):\n {e}" ) contract.convert_expression_to_slithir_ssa() diff --git a/tests/lookup/private_variable.sol b/tests/lookup/private_variable.sol index e69de29bb2..93f4d3dda6 100644 --- a/tests/lookup/private_variable.sol +++ b/tests/lookup/private_variable.sol @@ -0,0 +1,13 @@ +contract A{ + uint private v = 10; +} + +contract B{ + uint v = 20; +} + +contract C is B, A{ + function f() public view returns(uint) { + return v; + } +} \ No newline at end of file diff --git a/tests/test_features.py b/tests/test_features.py index 924e0b1543..0303a5760b 100644 --- a/tests/test_features.py +++ b/tests/test_features.py @@ -5,6 +5,7 @@ from solc_select import solc_select from slither import Slither +from slither.core.variables.state_variable import StateVariable from slither.detectors import all_detectors from slither.detectors.abstract_detector import AbstractDetector from slither.slithir.operations import LibraryCall, InternalCall @@ -139,3 +140,13 @@ def test_using_for_in_library() -> None: if isinstance(ir, LibraryCall) and ir.destination == "B" and ir.function_name == "b": return assert False + + +def test_private_variable() -> None: + solc_select.switch_global_version("0.8.15", always_install=True) + slither = Slither("./tests/lookup/private_variable.sol") + contract_c = slither.get_contract_from_name("C")[0] + f = contract_c.functions[0] + var_read = f.variables_read[0] + assert isinstance(var_read, StateVariable) + assert str(var_read.contract) == "B"