From 21d8b21f0a0e71ff8c8d28e0745ab6895e7375b3 Mon Sep 17 00:00:00 2001
From: hazim-j <hazim.jumali@gmail.com>
Date: Tue, 5 Aug 2025 17:23:35 +1000
Subject: [PATCH 1/2] Findings 29: revert if more than max uint8 owners

---
 src/verifier/UserOpMultiSigVerifier.sol    |  2 ++
 test/verifier/UserOpMultiSigVerifier.t.sol | 38 +++++++++++++++++++---
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/src/verifier/UserOpMultiSigVerifier.sol b/src/verifier/UserOpMultiSigVerifier.sol
index 35e9ccb..dbad8ae 100644
--- a/src/verifier/UserOpMultiSigVerifier.sol
+++ b/src/verifier/UserOpMultiSigVerifier.sol
@@ -10,6 +10,7 @@ import {OnlyKeystore} from "../lib/OnlyKeystore.sol";
 
 contract UserOpMultiSigVerifier is IVerifier, OnlyKeystore {
     error ZeroThresholdNotAllowed();
+    error MaxOwnersLimitExceeded();
 
     bytes1 public constant SIGNATURES_ONLY_TAG = 0xff;
 
@@ -47,6 +48,7 @@ contract UserOpMultiSigVerifier is IVerifier, OnlyKeystore {
     {
         (uint8 threshold, address[] memory owners) = abi.decode(config, (uint8, address[]));
         require(threshold > 0, ZeroThresholdNotAllowed());
+        require(owners.length <= type(uint8).max, MaxOwnersLimitExceeded());
 
         SignerData[] memory signatures;
         if (bytes1(data[0]) == SIGNATURES_ONLY_TAG) {
diff --git a/test/verifier/UserOpMultiSigVerifier.t.sol b/test/verifier/UserOpMultiSigVerifier.t.sol
index 0c1aa4c..f9e3dfb 100644
--- a/test/verifier/UserOpMultiSigVerifier.t.sol
+++ b/test/verifier/UserOpMultiSigVerifier.t.sol
@@ -82,6 +82,26 @@ contract UserOpMultiSigVerifierTest is Test {
         verifier.validateData(message, data, config);
     }
 
+    function testFuzz_validateDataMaxOwners(bool withUserOp, uint8 threshold, uint8 offset, uint8 excess) public {
+        uint16 size = _getSizeAndAssumeMaxOwnerLimitExceeded(threshold, offset, excess);
+        Signer[] memory signers = _createSigners(size);
+
+        bytes32 message = keccak256("Signed by signer");
+        bytes memory data = _createData(message, threshold, offset, signers);
+        if (withUserOp) {
+            PackedUserOperation memory userOp;
+            userOp.signature = data;
+            data = abi.encode(userOp);
+        } else {
+            data = abi.encodePacked(verifier.SIGNATURES_ONLY_TAG(), data);
+        }
+
+        bytes memory config = _createConfig(threshold, signers);
+
+        vm.expectRevert(UserOpMultiSigVerifier.MaxOwnersLimitExceeded.selector);
+        verifier.validateData(message, data, config);
+    }
+
     function testFuzz_validateDataInvalidCaller(address keystore) public {
         vm.assume(keystore != address(this));
         vm.prank(keystore);
@@ -103,9 +123,19 @@ contract UserOpMultiSigVerifierTest is Test {
         vm.assume(uint16(threshold) + uint16(offset) <= size);
     }
 
-    function _createSigners(uint8 size) internal returns (Signer[] memory) {
+    function _getSizeAndAssumeMaxOwnerLimitExceeded(uint8 threshold, uint8 offset, uint8 excess)
+        internal
+        pure
+        returns (uint16 size)
+    {
+        size = uint16(type(uint8).max) + excess;
+        vm.assume(threshold > 0 && excess > 0);
+        vm.assume(uint16(threshold) + uint16(offset) <= size);
+    }
+
+    function _createSigners(uint16 size) internal returns (Signer[] memory) {
         Signer[] memory signers = new Signer[](size);
-        for (uint8 i = 0; i < size; i++) {
+        for (uint16 i = 0; i < size; i++) {
             (address addr, uint256 pk) = makeAddrAndKey(LibString.toString(i));
             signers[i] = Signer({addr: addr, pk: pk});
         }
@@ -127,9 +157,9 @@ contract UserOpMultiSigVerifierTest is Test {
         returns (bytes memory)
     {
         UserOpMultiSigVerifier.SignerData[] memory sd = new UserOpMultiSigVerifier.SignerData[](threshold);
-        for (uint8 i = 0; i < threshold; i++) {
+        for (uint16 i = 0; i < threshold; i++) {
             (uint8 v, bytes32 r, bytes32 s) = vm.sign(signers[i + offset].pk, message);
-            sd[i] = UserOpMultiSigVerifier.SignerData({index: i + offset, signature: abi.encodePacked(r, s, v)});
+            sd[i] = UserOpMultiSigVerifier.SignerData({index: uint8(i + offset), signature: abi.encodePacked(r, s, v)});
         }
 
         return abi.encode(sd);

From 50258437c9d2b4070875acbb276434129f3c725b Mon Sep 17 00:00:00 2001
From: hazim-j <hazim.jumali@gmail.com>
Date: Tue, 5 Aug 2025 17:42:10 +1000
Subject: [PATCH 2/2] add comment

---
 test/verifier/UserOpMultiSigVerifier.t.sol | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/test/verifier/UserOpMultiSigVerifier.t.sol b/test/verifier/UserOpMultiSigVerifier.t.sol
index f9e3dfb..107d745 100644
--- a/test/verifier/UserOpMultiSigVerifier.t.sol
+++ b/test/verifier/UserOpMultiSigVerifier.t.sol
@@ -157,9 +157,15 @@ contract UserOpMultiSigVerifierTest is Test {
         returns (bytes memory)
     {
         UserOpMultiSigVerifier.SignerData[] memory sd = new UserOpMultiSigVerifier.SignerData[](threshold);
-        for (uint16 i = 0; i < threshold; i++) {
-            (uint8 v, bytes32 r, bytes32 s) = vm.sign(signers[i + offset].pk, message);
-            sd[i] = UserOpMultiSigVerifier.SignerData({index: uint8(i + offset), signature: abi.encodePacked(r, s, v)});
+        for (uint8 i = 0; i < threshold; i++) {
+            uint16 index = uint16(i) + offset;
+            (uint8 v, bytes32 r, bytes32 s) = vm.sign(signers[index].pk, message);
+            sd[i] = UserOpMultiSigVerifier.SignerData({
+                // Note: index will overflow back to 0 after max uint8.
+                // This is ok since a MaxOwnersLimitExceeded() error is expected.
+                index: uint8(index),
+                signature: abi.encodePacked(r, s, v)
+            });
         }
 
         return abi.encode(sd);