From 7642299d3944372cd8293a890dce92a449c76022 Mon Sep 17 00:00:00 2001
From: Simone <simone.monica@trailofbits.com>
Date: Fri, 6 Dec 2024 17:15:54 +0100
Subject: [PATCH] Fix reorder argument edge case

---
 slither/slithir/convert.py                       | 12 +++++++++---
 tests/unit/slithir/test_argument_reorder.py      | 16 ++++++++++++----
 .../test_overridden_function.sol                 |  6 +++---
 3 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/slither/slithir/convert.py b/slither/slithir/convert.py
index c22095226..cc27eb286 100644
--- a/slither/slithir/convert.py
+++ b/slither/slithir/convert.py
@@ -433,8 +433,11 @@ def reorder_arguments(
         assert len(call_names) == len(names)
 
     args_ret = []
-    index_seen = []
+    index_seen = []  # Will have the correct index order
 
+    # We search for declaration of parameters that is fully compatible with the call arguments
+    # that is why if a arg name is not present in the call name we break and clear index_seen
+    # Known issue if the overridden function reuse the same parameters' name but in different positions
     for names in decl_names:
         if len(index_seen) == len(args):
             break
@@ -445,9 +448,12 @@ def reorder_arguments(
                 if ind in index_seen:
                     continue
             except ValueError:
-                continue
+                index_seen.clear()
+                break
             index_seen.append(ind)
-            args_ret.append(args[ind])
+
+    for ind in index_seen:
+        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 5d1142694..f08314afe 100644
--- a/tests/unit/slithir/test_argument_reorder.py
+++ b/tests/unit/slithir/test_argument_reorder.py
@@ -77,11 +77,15 @@ def test_overridden_function_reorder(solc_binary_path) -> None:
 
     assert (
         isinstance(internal_calls[0].arguments[0], Constant)
-        and internal_calls[0].arguments[0].value == 34
+        and internal_calls[0].arguments[0].value == 23
     )
     assert (
         isinstance(internal_calls[0].arguments[1], Constant)
-        and internal_calls[0].arguments[1].value == 23
+        and internal_calls[0].arguments[1].value == 36
+    )
+    assert (
+        isinstance(internal_calls[0].arguments[2], Constant)
+        and internal_calls[0].arguments[2].value == 34
     )
 
     operations = slither.contracts[1].functions[1].slithir_operations
@@ -90,9 +94,13 @@ def test_overridden_function_reorder(solc_binary_path) -> None:
 
     assert (
         isinstance(internal_calls[0].arguments[0], Constant)
-        and internal_calls[0].arguments[0].value == 34
+        and internal_calls[0].arguments[0].value == 23
     )
     assert (
         isinstance(internal_calls[0].arguments[1], Constant)
-        and internal_calls[0].arguments[1].value == 23
+        and internal_calls[0].arguments[1].value == 36
+    )
+    assert (
+        isinstance(internal_calls[0].arguments[2], Constant)
+        and internal_calls[0].arguments[2].value == 34
     )
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
index 2be5e45a1..4baf0df65 100644
--- a/tests/unit/slithir/test_data/argument_reorder/test_overridden_function.sol
+++ b/tests/unit/slithir/test_data/argument_reorder/test_overridden_function.sol
@@ -1,8 +1,8 @@
 contract A {
- function a(uint256 q, uint256 e) internal virtual {}
- function b() public { a({e: 23, q: 34}); }
+ function a(uint256 e, uint256 q, uint256 w) internal virtual {}
+ function b() public { a({e: 23, w: 34, q: 36}); }
 }
 
 contract B is A {
- function a(uint256 w, uint256 q) internal override {}
+ function a(uint256 q, uint256 ww, uint256 e) internal override {}
 }
\ No newline at end of file