From e11580f188f3a786f5df07ba24f8fa98c3793dae Mon Sep 17 00:00:00 2001 From: Judy Wu Date: Sat, 7 Oct 2023 23:44:01 -0400 Subject: [PATCH 1/2] fix: resolve state variable access in inherited internal functions from derived contracts, closes #1654 --- slither/solc_parsing/yul/parse_yul.py | 2 +- .../test_data/assembly_storage_slot.sol | 18 +++++++++ .../slithir/test_yul_parser_assembly_slot.py | 39 +++++++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 tests/unit/slithir/test_data/assembly_storage_slot.sol create mode 100644 tests/unit/slithir/test_yul_parser_assembly_slot.py diff --git a/slither/solc_parsing/yul/parse_yul.py b/slither/solc_parsing/yul/parse_yul.py index 8657947eaf..cf942ea59c 100644 --- a/slither/solc_parsing/yul/parse_yul.py +++ b/slither/solc_parsing/yul/parse_yul.py @@ -748,7 +748,7 @@ def parse_yul_function_call(root: YulScope, node: YulNode, ast: Dict) -> Optiona def _check_for_state_variable_name(root: YulScope, potential_name: str) -> Optional[Identifier]: root_function = root.function if isinstance(root_function, FunctionContract): - var = root_function.contract.get_state_variable_from_name(potential_name) + var = root_function.contract_declarer.get_state_variable_from_name(potential_name) if var: return Identifier(var) return None diff --git a/tests/unit/slithir/test_data/assembly_storage_slot.sol b/tests/unit/slithir/test_data/assembly_storage_slot.sol new file mode 100644 index 0000000000..244f24fe6c --- /dev/null +++ b/tests/unit/slithir/test_data/assembly_storage_slot.sol @@ -0,0 +1,18 @@ +contract YYY { + mapping(address => uint256) private _counters; + function _getPackedBucketGlobalState(uint256 bucketId) internal view returns (uint256 packedGlobalState) { + assembly { + mstore(0x0, bucketId) + mstore(0x20, _counters.slot) + let slot := keccak256(0x0, 0x40) + packedGlobalState := sload(slot) + } + } +} + + +contract XXX is YYY { + function getPackedBucketGlobalState(uint256 bucketId) external { + _getPackedBucketGlobalState(bucketId); + } +} \ No newline at end of file diff --git a/tests/unit/slithir/test_yul_parser_assembly_slot.py b/tests/unit/slithir/test_yul_parser_assembly_slot.py new file mode 100644 index 0000000000..d1bb5bb40d --- /dev/null +++ b/tests/unit/slithir/test_yul_parser_assembly_slot.py @@ -0,0 +1,39 @@ +from pathlib import Path +from slither import Slither + +from slither.core.expressions import CallExpression +from slither.core.expressions.identifier import Identifier +from slither.core.expressions.literal import Literal +from slither.core.variables.state_variable import StateVariable +from slither.core.variables.local_variable import LocalVariable + + +TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data" + + +def test_yul_parser_assembly_slot(solc_binary_path) -> None: + # mstore(0x0, bucketId) + # mstore(0x20, _counters.slot) + data = { + '0x0': 'bucketId', + '0x20': '_counters' + } + + solc_path = solc_binary_path("0.8.18") + slither = Slither(Path(TEST_DATA_DIR, "assembly_storage_slot.sol").as_posix(), solc=solc_path) + + contract = slither.get_contract_from_name('XXX')[0] + func = contract.get_function_from_full_name('getPackedBucketGlobalState(uint256)') + calls = [node.expression for node in func.all_nodes() if node.expression and 'mstore' in str(node.expression)] + + for call in calls: + assert isinstance(call, CallExpression) + memory_location = call.arguments[0] + value = call.arguments[1] + assert isinstance(memory_location, Literal) + assert isinstance(value, Identifier) + assert value.value.name == data[memory_location.value] + if value.value.name == '_counters': + assert isinstance(value.value, StateVariable) + elif value.value.name == 'bucketId': + assert isinstance(value.value, LocalVariable) From 7f5535fc2411aae9db8b28a9855255745f64c028 Mon Sep 17 00:00:00 2001 From: Feist Josselin Date: Fri, 13 Oct 2023 11:41:06 +0200 Subject: [PATCH 2/2] Run black on the test file --- .../slithir/test_yul_parser_assembly_slot.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/tests/unit/slithir/test_yul_parser_assembly_slot.py b/tests/unit/slithir/test_yul_parser_assembly_slot.py index d1bb5bb40d..da800b55e0 100644 --- a/tests/unit/slithir/test_yul_parser_assembly_slot.py +++ b/tests/unit/slithir/test_yul_parser_assembly_slot.py @@ -14,17 +14,18 @@ def test_yul_parser_assembly_slot(solc_binary_path) -> None: # mstore(0x0, bucketId) # mstore(0x20, _counters.slot) - data = { - '0x0': 'bucketId', - '0x20': '_counters' - } + data = {"0x0": "bucketId", "0x20": "_counters"} solc_path = solc_binary_path("0.8.18") slither = Slither(Path(TEST_DATA_DIR, "assembly_storage_slot.sol").as_posix(), solc=solc_path) - contract = slither.get_contract_from_name('XXX')[0] - func = contract.get_function_from_full_name('getPackedBucketGlobalState(uint256)') - calls = [node.expression for node in func.all_nodes() if node.expression and 'mstore' in str(node.expression)] + contract = slither.get_contract_from_name("XXX")[0] + func = contract.get_function_from_full_name("getPackedBucketGlobalState(uint256)") + calls = [ + node.expression + for node in func.all_nodes() + if node.expression and "mstore" in str(node.expression) + ] for call in calls: assert isinstance(call, CallExpression) @@ -33,7 +34,7 @@ def test_yul_parser_assembly_slot(solc_binary_path) -> None: assert isinstance(memory_location, Literal) assert isinstance(value, Identifier) assert value.value.name == data[memory_location.value] - if value.value.name == '_counters': + if value.value.name == "_counters": assert isinstance(value.value, StateVariable) - elif value.value.name == 'bucketId': + elif value.value.name == "bucketId": assert isinstance(value.value, LocalVariable)