Merge pull request #2611 from crytic/dev-fix-override-reorder

Fix reorder arguments when a function is overridden with diff param names

slither/slithir/convert.py

@@ -380,34 +380,39 @@ 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.
     #### Parameters
     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
 

tests/unit/slithir/test_argument_reorder.py

@@ -63,3 +63,36 @@ 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
+    )

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 {}
+}