Findings 30: revert if more than max uint8 signatures

src/verifier/UserOpMultiSigVerifier.sol

@@ -11,6 +11,7 @@ import {OnlyKeystore} from "../lib/OnlyKeystore.sol";
 contract UserOpMultiSigVerifier is IVerifier, OnlyKeystore {
     error ZeroThresholdNotAllowed();
     error MaxOwnersLimitExceeded();
+    error MaxSignaturesExceeded();
 
     bytes1 public constant SIGNATURES_ONLY_TAG = 0xff;
 
@@ -57,11 +58,12 @@ contract UserOpMultiSigVerifier is IVerifier, OnlyKeystore {
             PackedUserOperation memory userOp = abi.decode(data, (PackedUserOperation));
             signatures = abi.decode(userOp.signature, (SignerData[]));
         }
+        uint256 length = signatures.length;
+        require(length <= type(uint8).max, MaxSignaturesExceeded());
 
         uint8 valid = 0;
         uint8 invalid = 0;
         bool[] memory seen = new bool[](owners.length);
-        uint256 length = signatures.length;
         for (uint256 i = 0; i < length; i++) {
             SignerData memory sd = signatures[i];
 

test/verifier/UserOpMultiSigVerifier.t.sol

@@ -102,6 +102,71 @@ contract UserOpMultiSigVerifierTest is Test {
         verifier.validateData(message, data, config);
     }
 
+    function testFuzz_validateDataDuplicateSignatures(bool withUserOp, uint8 threshold, uint8 dup) public {
+        // Note: set threshold > 1 to show we can't recycle the same signature
+        // multiple times.
+        vm.assume(threshold > 1);
+        vm.assume(dup >= 1);
+
+        Signer[] memory signers = _createSigners(1);
+        bytes32 message = keccak256("Signed by signer");
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(signers[0].pk, message);
+        bytes memory signature = abi.encodePacked(r, s, v);
+
+        UserOpMultiSigVerifier.SignerData[] memory sd = new UserOpMultiSigVerifier.SignerData[](dup);
+        for (uint8 i = 0; i < dup; i++) {
+            sd[i] = UserOpMultiSigVerifier.SignerData({index: 0, signature: signature});
+        }
+
+        bytes memory data = abi.encode(sd);
+        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);
+
+        uint256 validationData = verifier.validateData(message, data, config);
+        assertEq(validationData, SIG_VALIDATION_FAILED);
+    }
+
+    function testFuzz_validateDataMaxSignatures(bool withUserOp, uint8 excess) public {
+        vm.assume(excess > 0);
+
+        Signer[] memory signers = _createSigners(1);
+        bytes32 message = keccak256("Signed by signer");
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(signers[0].pk, message);
+        bytes memory signature = abi.encodePacked(r, s, v);
+
+        uint16 count = uint16(type(uint8).max) + excess;
+        UserOpMultiSigVerifier.SignerData[] memory sd = new UserOpMultiSigVerifier.SignerData[](count);
+        for (uint16 i = 0; i < count; i++) {
+            sd[i] = UserOpMultiSigVerifier.SignerData({
+                // Note: index will overflow back to 0 after max uint8.
+                // This is ok since a MaxSignaturesExceeded() error is expected.
+                index: 0,
+                signature: signature
+            });
+        }
+
+        bytes memory data = abi.encode(sd);
+        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(1, signers);
+
+        vm.expectRevert(UserOpMultiSigVerifier.MaxSignaturesExceeded.selector);
+        verifier.validateData(message, data, config);
+    }
+
     function testFuzz_validateDataInvalidCaller(address keystore) public {
         vm.assume(keystore != address(this));
         vm.prank(keystore);