diff --git a/src/account/UpgradeableModularAccount.sol b/src/account/UpgradeableModularAccount.sol index 12f67fa2..e05cb016 100644 --- a/src/account/UpgradeableModularAccount.sol +++ b/src/account/UpgradeableModularAccount.sol @@ -73,6 +73,7 @@ contract UpgradeableModularAccount is error UserOpValidationFunctionMissing(bytes4 selector); error ValidationDoesNotApply(bytes4 selector, address plugin, uint8 functionId, bool isDefault); error SignatureSegmentOutOfOrder(); + error NonCanonicalEncoding(); error ValidationSignatureSegmentMissing(); // Wraps execution of a native function with runtime validation and hooks @@ -374,6 +375,10 @@ contract UpgradeableModularAccount is // Use the current segment userOp.signature = signatureSegment.getBody(); + if (userOp.signature.length == 0) { + revert NonCanonicalEncoding(); + } + // Load the next per-hook data segment (signatureSegment, signature) = signature.getNextSegment(); @@ -437,6 +442,10 @@ contract UpgradeableModularAccount is // Use the current segment currentAuthData = authSegment.getBody(); + if (currentAuthData.length == 0) { + revert NonCanonicalEncoding(); + } + // Load the next per-hook data segment (authSegment, authorizationData) = authorizationData.getNextSegment(); diff --git a/test/account/PerHookData.t.sol b/test/account/PerHookData.t.sol index a7542c0c..f97951b1 100644 --- a/test/account/PerHookData.t.sol +++ b/test/account/PerHookData.t.sol @@ -194,6 +194,30 @@ contract PerHookDataTest is AccountTestBase { entryPoint.handleOps(userOps, beneficiary); } + function test_failPerHookData_nonCanonicalEncoding_userOp() public { + (PackedUserOperation memory userOp, bytes32 userOpHash) = _getCounterUserOP(); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, userOpHash.toEthSignedMessageHash()); + + PreValidationHookData[] memory preValidationHookData = new PreValidationHookData[](1); + preValidationHookData[0] = PreValidationHookData({index: 0, validationData: ""}); + + userOp.signature = + _encodeSignature(ownerValidation, DEFAULT_VALIDATION, preValidationHookData, abi.encodePacked(r, s, v)); + + PackedUserOperation[] memory userOps = new PackedUserOperation[](1); + userOps[0] = userOp; + + vm.expectRevert( + abi.encodeWithSelector( + IEntryPoint.FailedOpWithRevert.selector, + 0, + "AA23 reverted", + abi.encodeWithSelector(UpgradeableModularAccount.NonCanonicalEncoding.selector) + ) + ); + entryPoint.handleOps(userOps, beneficiary); + } + function test_passAccessControl_runtime() public { assertEq(counter.number(), 0); @@ -291,6 +315,20 @@ contract PerHookDataTest is AccountTestBase { ); } + function test_failPerHookData_nonCanonicalEncoding_runtime() public { + PreValidationHookData[] memory preValidationHookData = new PreValidationHookData[](1); + preValidationHookData[0] = PreValidationHookData({index: 0, validationData: ""}); + + vm.prank(owner1); + vm.expectRevert(abi.encodeWithSelector(UpgradeableModularAccount.NonCanonicalEncoding.selector)); + account1.executeWithAuthorization( + abi.encodeCall( + UpgradeableModularAccount.execute, (address(counter), 0 wei, abi.encodeCall(Counter.increment, ())) + ), + _encodeSignature(ownerValidation, DEFAULT_VALIDATION, preValidationHookData, "") + ); + } + function _getCounterUserOP() internal view returns (PackedUserOperation memory, bytes32) { PackedUserOperation memory userOp = PackedUserOperation({ sender: address(account1),