From 2c401e6ca30bc154e6941c5871fa3b6886b344c8 Mon Sep 17 00:00:00 2001 From: Simone Date: Tue, 3 Dec 2024 18:05:29 +0100 Subject: [PATCH 1/2] Fix reorder arguments when a function is overridden with diff param names --- slither/slithir/convert.py | 42 +++++++++++++------ tests/unit/slithir/test_argument_reorder.py | 32 ++++++++++++++ .../test_overridden_function.sol | 8 ++++ 3 files changed, 70 insertions(+), 12 deletions(-) create mode 100644 tests/unit/slithir/test_data/argument_reorder/test_overridden_function.sol diff --git a/slither/slithir/convert.py b/slither/slithir/convert.py index e4798cced5..c220952261 100644 --- a/slither/slithir/convert.py +++ b/slither/slithir/convert.py @@ -380,7 +380,7 @@ def get_declared_param_names( InternalDynamicCall, EventCall, ] -) -> Optional[List[str]]: +) -> Optional[List[List[str]]]: """ Given a call operation, return the list of parameter names, in the order listed in the function declaration. @@ -388,26 +388,31 @@ def get_declared_param_names( ins - The call instruction #### Possible Returns - List[str] - - A list of the parameters in declaration order + List[List[str]] - + A list of list of parameters in declaration order. Note only if it's a Function there can be multiple list None - Workaround: Unable to obtain list of parameters in declaration order """ if isinstance(ins, NewStructure): - return [x.name for x in ins.structure.elems_ordered if not isinstance(x.type, MappingType)] + return [ + [x.name for x in ins.structure.elems_ordered if not isinstance(x.type, MappingType)] + ] if isinstance(ins, (InternalCall, LibraryCall, HighLevelCall)): if isinstance(ins.function, Function): - return [p.name for p in ins.function.parameters] + res = [[p.name for p in ins.function.parameters]] + for f in ins.function.overrides: + res.append([p.name for p in f.parameters]) + return res return None if isinstance(ins, InternalDynamicCall): - return [p.name for p in ins.function_type.params] + return [[p.name for p in ins.function_type.params]] assert isinstance(ins, (EventCall, NewContract)) return None def reorder_arguments( - args: List[Variable], call_names: List[str], decl_names: List[str] + args: List[Variable], call_names: List[str], decl_names: List[List[str]] ) -> List[Variable]: """ Reorder named struct constructor arguments so that they match struct declaration ordering rather @@ -419,17 +424,30 @@ def reorder_arguments( names - Parameter names in call order decl_names - - Parameter names in declaration order + List of list of parameter names in declaration order #### Returns Reordered arguments to constructor call, now in declaration order """ assert len(args) == len(call_names) - assert len(call_names) == len(decl_names) + for names in decl_names: + assert len(call_names) == len(names) args_ret = [] - for n in decl_names: - ind = call_names.index(n) - args_ret.append(args[ind]) + index_seen = [] + + for names in decl_names: + if len(index_seen) == len(args): + break + + for n in names: + try: + ind = call_names.index(n) + if ind in index_seen: + continue + except ValueError: + continue + index_seen.append(ind) + args_ret.append(args[ind]) return args_ret diff --git a/tests/unit/slithir/test_argument_reorder.py b/tests/unit/slithir/test_argument_reorder.py index 12f5bd7f28..0743bf6949 100644 --- a/tests/unit/slithir/test_argument_reorder.py +++ b/tests/unit/slithir/test_argument_reorder.py @@ -63,3 +63,35 @@ def test_internal_call_reorder(solc_binary_path) -> None: isinstance(internal_calls[0].arguments[2], Constant) and internal_calls[0].arguments[2].value == 5 ) + +def test_overridden_function_reorder(solc_binary_path) -> None: + solc_path = solc_binary_path("0.8.15") + slither = Slither( + Path(ARG_REORDER_TEST_ROOT, "test_overridden_function.sol").as_posix(), solc=solc_path + ) + + operations = slither.contracts[0].functions[1].slithir_operations + internal_calls = [x for x in operations if isinstance(x, InternalCall)] + assert len(internal_calls) == 1 + + assert ( + isinstance(internal_calls[0].arguments[0], Constant) + and internal_calls[0].arguments[0].value == 34 + ) + assert ( + isinstance(internal_calls[0].arguments[1], Constant) + and internal_calls[0].arguments[1].value == 23 + ) + + operations = slither.contracts[1].functions[1].slithir_operations + internal_calls = [x for x in operations if isinstance(x, InternalCall)] + assert len(internal_calls) == 1 + + assert ( + isinstance(internal_calls[0].arguments[0], Constant) + and internal_calls[0].arguments[0].value == 34 + ) + assert ( + isinstance(internal_calls[0].arguments[1], Constant) + and internal_calls[0].arguments[1].value == 23 + ) diff --git a/tests/unit/slithir/test_data/argument_reorder/test_overridden_function.sol b/tests/unit/slithir/test_data/argument_reorder/test_overridden_function.sol new file mode 100644 index 0000000000..2be5e45a15 --- /dev/null +++ b/tests/unit/slithir/test_data/argument_reorder/test_overridden_function.sol @@ -0,0 +1,8 @@ +contract A { + function a(uint256 q, uint256 e) internal virtual {} + function b() public { a({e: 23, q: 34}); } +} + +contract B is A { + function a(uint256 w, uint256 q) internal override {} +} \ No newline at end of file From c97abac28dbcf1f7974524dbec30c7c318bc906e Mon Sep 17 00:00:00 2001 From: Simone <79767264+smonicas@users.noreply.github.com> Date: Tue, 3 Dec 2024 18:13:50 +0100 Subject: [PATCH 2/2] Update tests/unit/slithir/test_argument_reorder.py Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- tests/unit/slithir/test_argument_reorder.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/slithir/test_argument_reorder.py b/tests/unit/slithir/test_argument_reorder.py index 0743bf6949..5d1142694f 100644 --- a/tests/unit/slithir/test_argument_reorder.py +++ b/tests/unit/slithir/test_argument_reorder.py @@ -64,6 +64,7 @@ def test_internal_call_reorder(solc_binary_path) -> None: and internal_calls[0].arguments[2].value == 5 ) + def test_overridden_function_reorder(solc_binary_path) -> None: solc_path = solc_binary_path("0.8.15") slither = Slither(