enforce canonical encoding

erc6900 · Jun 20, 2024 · 8a09d31 · 8a09d31
1 parent bd08139
commit 8a09d31
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 0 deletions.
diff --git 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
@@ -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),