diff --git a/src/verifier/UserOpMultiSigVerifier.sol b/src/verifier/UserOpMultiSigVerifier.sol index dbad8ae..0844b25 100644 --- a/src/verifier/UserOpMultiSigVerifier.sol +++ b/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]; diff --git a/test/verifier/UserOpMultiSigVerifier.t.sol b/test/verifier/UserOpMultiSigVerifier.t.sol index 107d745..cc7eed3 100644 --- a/test/verifier/UserOpMultiSigVerifier.t.sol +++ b/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);