feat(SingleSignerValidation): add replay safe hash and utility contracts for EIP-712 in modules [2/2] (#141)

Author: adamegyed

src/modules/ModuleEIP712.sol

@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity ^0.8.25;
+
+// A base for modules that use EIP-712 structured data signing.
+//
+// Unlike other EIP712 libraries, this mixin uses the salt field to hold the account address.
+//
+// It omits the name and version from the EIP-712 domain, as modules are intended to be deployed as
+// immutable singletons, thus a different versions and instances would have a different module address.
+//
+// Due to depending on the account address to calculate the domain separator, this abstract contract does not
+// implement EIP-5267, as the domain retrieval function does not provide a parameter to use for the account address
+// (internally the verifyingContract), and the computed `msg.sender` for an `eth_call` without an override is
+// address(0).
+abstract contract ModuleEIP712 {
+    // keccak256(
+    //     "EIP712Domain(uint256 chainId,address verifyingContract,bytes32 salt)"
+    // )
+    bytes32 private constant _DOMAIN_SEPARATOR_TYPEHASH =
+        0x71062c282d40422f744945d587dbf4ecfd4f9cfad1d35d62c944373009d96162;
+
+    function _domainSeparator(address account) internal view returns (bytes32) {
+        return keccak256(
+            abi.encode(
+                _DOMAIN_SEPARATOR_TYPEHASH, block.chainid, address(this), bytes32(uint256(uint160(account)))
+            )
+        );
+    }
+}

src/modules/ReplaySafeWrapper.sol

@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity ^0.8.25;
+
+import {ModuleEIP712} from "./ModuleEIP712.sol";
+
+import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol";
+
+// A contract mixin for modules that wish to use EIP-712 to wrap the hashes sent to the EIP-1271 function
+// `isValidSignature`.
+// This makes signatures generated by owners of contract accounts non-replayable across multiple accounts owned by
+// the same owner.
+abstract contract ReplaySafeWrapper is ModuleEIP712 {
+    // keccak256("ReplaySafeHash(bytes32 hash)")
+    bytes32 private constant _REPLAY_SAFE_HASH_TYPEHASH =
+        0x294a8735843d4afb4f017c76faf3b7731def145ed0025fc9b1d5ce30adf113ff;
+
+    /// @notice Wraps a hash in an EIP-712 envelope to prevent cross-account replay attacks.
+    /// Uses the ModuleEIP712 domain separator, which includes the chainId, module address, and account address.
+    /// @param account The account that will validate the message hash.
+    /// @param hash The hash to wrap.
+    /// @return The the replay-safe hash, computed by wrapping the input hash in an EIP-712 struct.
+    function replaySafeHash(address account, bytes32 hash) public view virtual returns (bytes32) {
+        return MessageHashUtils.toTypedDataHash({
+            domainSeparator: _domainSeparator(account),
+            structHash: _hashStruct(hash)
+        });
+    }
+
+    function _hashStruct(bytes32 hash) internal view virtual returns (bytes32) {
+        return keccak256(abi.encode(_REPLAY_SAFE_HASH_TYPEHASH, hash));
+    }
+}

src/modules/validation/SingleSignerValidationModule.sol

@@ -4,6 +4,8 @@ pragma solidity ^0.8.25;
 import {IModule, ModuleMetadata} from "../../interfaces/IModule.sol";
 import {IValidationModule} from "../../interfaces/IValidationModule.sol";
 import {BaseModule} from "../BaseModule.sol";
+
+import {ReplaySafeWrapper} from "../ReplaySafeWrapper.sol";
 import {ISingleSignerValidationModule} from "./ISingleSignerValidationModule.sol";
 import {PackedUserOperation} from "@eth-infinitism/account-abstraction/interfaces/PackedUserOperation.sol";
 import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol";
@@ -17,12 +19,11 @@ import {SignatureChecker} from "@openzeppelin/contracts/utils/cryptography/Signa
 /// the account. Account states are to be retrieved from this global singleton directly.
 ///
 /// - This validation supports ERC-1271. The signature is valid if it is signed by the owner's private key
-/// (if the owner is an EOA) or if it is a valid ERC-1271 signature from the
-/// owner (if the owner is a contract).
+/// (if the owner is an EOA) or if it is a valid ERC-1271 signature from the owner (if the owner is a contract).
 ///
 /// - This validation supports composition that other validation can relay on entities in this validation
 /// to validate partially or fully.
-contract SingleSignerValidationModule is ISingleSignerValidationModule, BaseModule {
+contract SingleSignerValidationModule is ISingleSignerValidationModule, ReplaySafeWrapper, BaseModule {
     using MessageHashUtils for bytes32;
 
     string internal constant _NAME = "SingleSigner Validation";
@@ -93,14 +94,17 @@ contract SingleSignerValidationModule is ISingleSignerValidationModule, BaseModu
     /// @inheritdoc IValidationModule
     /// @dev The signature is valid if it is signed by the owner's private key
     /// (if the owner is an EOA) or if it is a valid ERC-1271 signature from the
-    /// owner (if the owner is a contract). Note that the signature is wrapped in an EIP-191 message
+    /// owner (if the owner is a contract).
+    /// Note that the digest is wrapped in an EIP-712 struct to prevent cross-account replay attacks. The
+    /// replay-safe hash may be retrieved by calling the public function `replaySafeHash`.
     function validateSignature(address account, uint32 entityId, address, bytes32 digest, bytes calldata signature)
         external
         view
         override
         returns (bytes4)
     {
-        if (SignatureChecker.isValidSignatureNow(signers[entityId][account], digest, signature)) {
+        bytes32 _replaySafeHash = replaySafeHash(account, digest);
+        if (SignatureChecker.isValidSignatureNow(signers[entityId][account], _replaySafeHash, signature)) {
             return _1271_MAGIC_VALUE;
         }
         return _1271_INVALID;

test/account/PerHookData.t.sol

@@ -23,9 +23,6 @@ contract PerHookDataTest is CustomValidationTestBase {
     Counter internal _counter;
 
     function setUp() public {
-        _signerValidation =
-            ModuleEntityLib.pack(address(singleSignerValidationModule), TEST_DEFAULT_VALIDATION_ENTITY_ID);
-
         _counter = new Counter();
 
         _accessControlHookModule = new MockAccessControlHookModule();
@@ -360,7 +357,8 @@ contract PerHookDataTest is CustomValidationTestBase {
 
         bytes32 messageHash = keccak256(abi.encodePacked(message));
 
-        (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, messageHash);
+        bytes32 replaySafeHash = singleSignerValidationModule.replaySafeHash(address(account1), messageHash);
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, replaySafeHash);
 
         PreValidationHookData[] memory preValidationHookData = new PreValidationHookData[](1);
         preValidationHookData[0] = PreValidationHookData({index: 0, validationData: abi.encodePacked(message)});

test/account/UpgradeableModularAccount.t.sol

@@ -392,9 +392,17 @@ contract UpgradeableModularAccountTest is AccountTestBase {
     function test_isValidSignature() public {
         bytes32 message = keccak256("hello world");
 
-        (uint8 v, bytes32 r, bytes32 s) = vm.sign(owner1Key, message);
+        uint8 v;
+        bytes32 r;
+        bytes32 s;
 
-        // singleSignerValidationModule.ownerOf(address(account1));
+        if (vm.envOr("SMA_TEST", false)) {
+            // todo: implement replay-safe hashing for SMA
+            (v, r, s) = vm.sign(owner1Key, message);
+        } else {
+            bytes32 replaySafeHash = singleSignerValidationModule.replaySafeHash(address(account1), message);
+            (v, r, s) = vm.sign(owner1Key, replaySafeHash);
+        }
 
         bytes memory signature = _encode1271Signature(_signerValidation, abi.encodePacked(r, s, v));
 

test/module/SingleSignerValidationModule.t.sol

@@ -113,8 +113,10 @@ contract SingleSignerValidationModuleTest is AccountTestBase {
 
         address accountAddr = address(account);
 
+        bytes32 replaySafeHash = singleSignerValidationModule.replaySafeHash(accountAddr, digest);
+
         vm.startPrank(accountAddr);
-        (uint8 v, bytes32 r, bytes32 s) = vm.sign(privateKey, digest);
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(privateKey, replaySafeHash);
 
         // sig check should fail
         assertEq(
@@ -141,7 +143,10 @@ contract SingleSignerValidationModuleTest is AccountTestBase {
         address accountAddr = address(account);
         vm.startPrank(accountAddr);
         singleSignerValidationModule.transferSigner(TEST_DEFAULT_VALIDATION_ENTITY_ID, address(contractOwner));
-        bytes memory signature = contractOwner.sign(digest);
+
+        bytes32 replaySafeHash = singleSignerValidationModule.replaySafeHash(accountAddr, digest);
+
+        bytes memory signature = contractOwner.sign(replaySafeHash);
         assertEq(
             singleSignerValidationModule.validateSignature(
                 accountAddr, TEST_DEFAULT_VALIDATION_ENTITY_ID, address(this), digest, signature