Secure handling of chainid in EIP712 domains

foundry.toml

@@ -6,5 +6,6 @@ fs_permissions = [{ access = "read", path = "./out"}]
 
 [fmt]
 line_length = 120
+multiline_func_header = 'all'
 
 # See more config options https://github.com/foundry-rs/foundry/tree/master/config

src/interfaces/IERC20Permit.sol

@@ -3,7 +3,15 @@
 pragma solidity 0.8.15;
 
 interface IERC20Permit {
-    function permit(address owner, address spender, uint256 value, uint256 deadline, uint8 v, bytes32 r, bytes32 s)
+    function permit(
+        address owner,
+        address spender,
+        uint256 value,
+        uint256 deadline,
+        uint8 v,
+        bytes32 r,
+        bytes32 s
+    )
         external;
 
     function nonces(address owner) external view returns (uint256);
@@ -14,7 +22,14 @@ interface IERC20Permit {
 
     function SALTED_PERMIT_TYPEHASH() external view returns (bytes32);
 
-    function receiveWithPermit(address _holder, uint256 _value, uint256 _deadline, uint8 _v, bytes32 _r, bytes32 _s)
+    function receiveWithPermit(
+        address _holder,
+        uint256 _value,
+        uint256 _deadline,
+        uint8 _v,
+        bytes32 _r,
+        bytes32 _s
+    )
         external;
 
     function saltedPermit(

src/token/ERC20Permit.sol

@@ -5,13 +5,12 @@ pragma solidity 0.8.15;
 import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
 import "../interfaces/IERC20Permit.sol";
 import "./BaseERC20.sol";
+import "../utils/EIP712.sol";
 
 /**
  * @title ERC20Permit
  */
-abstract contract ERC20Permit is BaseERC20, IERC20Permit {
-    // EIP712 domain separator
-    bytes32 public immutable DOMAIN_SEPARATOR;
+abstract contract ERC20Permit is IERC20Permit, BaseERC20, EIP712 {
     // EIP2612 permit typehash
     bytes32 public constant PERMIT_TYPEHASH =
         keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)");
@@ -23,16 +22,10 @@ abstract contract ERC20Permit is BaseERC20, IERC20Permit {
 
     mapping(address => uint256) public nonces;
 
-    constructor(address _self) {
-        DOMAIN_SEPARATOR = keccak256(
-            abi.encode(
-                keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"),
-                keccak256(bytes(name())),
-                keccak256("1"),
-                block.chainid,
-                _self
-            )
-        );
+    constructor(address _self) EIP712(_self, name(), "1") {}
+
+    function DOMAIN_SEPARATOR() external view override returns (bytes32) {
+        return _domainSeparatorV4();
     }
 
     /**
@@ -67,7 +60,14 @@ abstract contract ERC20Permit is BaseERC20, IERC20Permit {
     /**
      * @dev Cheap shortcut for making sequential calls to permit() + transferFrom() functions.
      */
-    function receiveWithPermit(address _holder, uint256 _value, uint256 _deadline, uint8 _v, bytes32 _r, bytes32 _s)
+    function receiveWithPermit(
+        address _holder,
+        uint256 _value,
+        uint256 _deadline,
+        uint8 _v,
+        bytes32 _r,
+        bytes32 _s
+    )
         public
         virtual
     {
@@ -140,7 +140,7 @@ abstract contract ERC20Permit is BaseERC20, IERC20Permit {
 
         uint256 nonce = nonces[_holder]++;
         bytes32 digest = ECDSA.toTypedDataHash(
-            DOMAIN_SEPARATOR, keccak256(abi.encode(PERMIT_TYPEHASH, _holder, _spender, _value, nonce, _deadline))
+            _domainSeparatorV4(), keccak256(abi.encode(PERMIT_TYPEHASH, _holder, _spender, _value, nonce, _deadline))
         );
 
         require(_holder == ECDSA.recover(digest, _v, _r, _s), "ERC20Permit: invalid ERC2612 signature");
@@ -162,7 +162,7 @@ abstract contract ERC20Permit is BaseERC20, IERC20Permit {
 
         uint256 nonce = nonces[_holder]++;
         bytes32 digest = ECDSA.toTypedDataHash(
-            DOMAIN_SEPARATOR,
+            _domainSeparatorV4(),
             keccak256(abi.encode(SALTED_PERMIT_TYPEHASH, _holder, _spender, _value, nonce, _deadline, _salt))
         );
 

src/utils/EIP712.sol

@@ -0,0 +1,107 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity 0.8.15;
+
+import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
+
+/**
+ * @dev https://eips.ethereum.org/EIPS/eip-712[EIP 712] is a standard for hashing and signing of typed structured data.
+ *
+ * The encoding specified in the EIP is very generic, and such a generic implementation in Solidity is not feasible,
+ * thus this contract does not implement the encoding itself. Protocols need to implement the type-specific encoding
+ * they need in their contracts using a combination of `abi.encode` and `keccak256`.
+ *
+ * This contract implements the EIP 712 domain separator ({_domainSeparatorV4}) that is used as part of the encoding
+ * scheme, and the final step of the encoding to obtain the message digest that is then signed via ECDSA
+ * ({_hashTypedDataV4}).
+ *
+ * The implementation of the domain separator was designed to be as efficient as possible while still properly updating
+ * the chain id to protect against replay attacks on an eventual fork of the chain.
+ *
+ * NOTE: This contract implements the version of the encoding known as "v4", as implemented by the JSON RPC method
+ * https://docs.metamask.io/guide/signing-data.html[`eth_signTypedDataV4` in MetaMask].
+ *
+ * Adapted from OpenZeppelin library to support address(this) overrides in proxy implementations.
+ */
+abstract contract EIP712 {
+    /* solhint-disable var-name-mixedcase */
+    // Cache the domain separator as an immutable value, but also store the chain id that it corresponds to, in order to
+    // invalidate the cached domain separator if the chain id changes.
+    bytes32 private immutable _CACHED_DOMAIN_SEPARATOR;
+    uint256 private immutable _CACHED_CHAIN_ID;
+    address private immutable _CACHED_THIS;
+
+    bytes32 private immutable _HASHED_NAME;
+    bytes32 private immutable _HASHED_VERSION;
+    bytes32 private immutable _TYPE_HASH;
+
+    /* solhint-enable var-name-mixedcase */
+
+    /**
+     * @dev Initializes the domain separator and parameter caches.
+     *
+     * The meaning of `name` and `version` is specified in
+     * https://eips.ethereum.org/EIPS/eip-712#definition-of-domainseparator[EIP 712]:
+     *
+     * - `name`: the user readable name of the signing domain, i.e. the name of the DApp or the protocol.
+     * - `version`: the current major version of the signing domain.
+     *
+     * NOTE: These parameters cannot be changed except through a xref:learn::upgrading-smart-contracts.adoc[smart
+     * contract upgrade].
+     */
+    constructor(address self, string memory name, string memory version) {
+        bytes32 hashedName = keccak256(bytes(name));
+        bytes32 hashedVersion = keccak256(bytes(version));
+        bytes32 typeHash =
+            keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)");
+        _HASHED_NAME = hashedName;
+        _HASHED_VERSION = hashedVersion;
+        _CACHED_CHAIN_ID = block.chainid;
+        _CACHED_DOMAIN_SEPARATOR = _buildDomainSeparator(typeHash, hashedName, hashedVersion, self);
+        _CACHED_THIS = self;
+        _TYPE_HASH = typeHash;
+    }
+
+    /**
+     * @dev Returns the domain separator for the current chain.
+     */
+    function _domainSeparatorV4() internal view returns (bytes32) {
+        if (address(this) == _CACHED_THIS && block.chainid == _CACHED_CHAIN_ID) {
+            return _CACHED_DOMAIN_SEPARATOR;
+        } else {
+            return _buildDomainSeparator(_TYPE_HASH, _HASHED_NAME, _HASHED_VERSION, address(this));
+        }
+    }
+
+    function _buildDomainSeparator(
+        bytes32 typeHash,
+        bytes32 nameHash,
+        bytes32 versionHash,
+        address self
+    )
+        private
+        view
+        returns (bytes32)
+    {
+        return keccak256(abi.encode(typeHash, nameHash, versionHash, block.chainid, self));
+    }
+
+    /**
+     * @dev Given an already https://eips.ethereum.org/EIPS/eip-712#definition-of-hashstruct[hashed struct], this
+     * function returns the hash of the fully encoded EIP712 message for this domain.
+     *
+     * This hash can be used together with {ECDSA-recover} to obtain the signer of a message. For example:
+     *
+     * ```solidity
+     * bytes32 digest = _hashTypedDataV4(keccak256(abi.encode(
+     *     keccak256("Mail(address to,string contents)"),
+     *     mailTo,
+     *     keccak256(bytes(mailContents))
+     * )));
+     * address signer = ECDSA.recover(digest, signature);
+     * ```
+     */
+    function _hashTypedDataV4(bytes32 structHash) internal view virtual returns (bytes32) {
+        return ECDSA.toTypedDataHash(_domainSeparatorV4(), structHash);
+    }
+}

src/zkbob/utils/ZkBobAccounting.sol

@@ -120,7 +120,10 @@ contract ZkBobAccounting {
         });
     }
 
-    function _recordOperation(address _user, int256 _txAmount)
+    function _recordOperation(
+        address _user,
+        int256 _txAmount
+    )
         internal
         returns (uint56 maxWeeklyAvgTvl, uint32 maxWeeklyTxCount, uint256 txCount)
     {

src/zkbob/verifiers/prodV1/TransferVerifier.sol

@@ -216,6 +216,8 @@ contract TransferVerifier {
             require(input[i] < SNARK_SCALAR_FIELD, "verifier-gte-snark-scalar-field");
             vk_x = Pairing.plus(vk_x, Pairing.scalar_mul(vk.IC[i + 1], input[i]));
         }
-        return Pairing.pairing(Pairing.negate(_proof.A), _proof.B, vk.alfa1, vk.beta2, vk_x, vk.gamma2, _proof.C, vk.delta2);
+        return Pairing.pairing(
+            Pairing.negate(_proof.A), _proof.B, vk.alfa1, vk.beta2, vk_x, vk.gamma2, _proof.C, vk.delta2
+        );
     }
 }

src/zkbob/verifiers/prodV1/TreeUpdateVerifier.sol

@@ -208,6 +208,8 @@ contract TreeUpdateVerifier {
             require(input[i] < SNARK_SCALAR_FIELD, "verifier-gte-snark-scalar-field");
             vk_x = Pairing.plus(vk_x, Pairing.scalar_mul(vk.IC[i + 1], input[i]));
         }
-        return Pairing.pairing(Pairing.negate(_proof.A), _proof.B, vk.alfa1, vk.beta2, vk_x, vk.gamma2, _proof.C, vk.delta2);
+        return Pairing.pairing(
+            Pairing.negate(_proof.A), _proof.B, vk.alfa1, vk.beta2, vk_x, vk.gamma2, _proof.C, vk.delta2
+        );
     }
 }

src/zkbob/verifiers/stageV1/TransferVerifier.sol

@@ -216,6 +216,8 @@ contract TransferVerifier {
             require(input[i] < SNARK_SCALAR_FIELD, "verifier-gte-snark-scalar-field");
             vk_x = Pairing.plus(vk_x, Pairing.scalar_mul(vk.IC[i + 1], input[i]));
         }
-        return Pairing.pairing(Pairing.negate(_proof.A), _proof.B, vk.alfa1, vk.beta2, vk_x, vk.gamma2, _proof.C, vk.delta2);
+        return Pairing.pairing(
+            Pairing.negate(_proof.A), _proof.B, vk.alfa1, vk.beta2, vk_x, vk.gamma2, _proof.C, vk.delta2
+        );
     }
 }

src/zkbob/verifiers/stageV1/TreeUpdateVerifier.sol

@@ -208,6 +208,8 @@ contract TreeUpdateVerifier {
             require(input[i] < SNARK_SCALAR_FIELD, "verifier-gte-snark-scalar-field");
             vk_x = Pairing.plus(vk_x, Pairing.scalar_mul(vk.IC[i + 1], input[i]));
         }
-        return Pairing.pairing(Pairing.negate(_proof.A), _proof.B, vk.alfa1, vk.beta2, vk_x, vk.gamma2, _proof.C, vk.delta2);
+        return Pairing.pairing(
+            Pairing.negate(_proof.A), _proof.B, vk.alfa1, vk.beta2, vk_x, vk.gamma2, _proof.C, vk.delta2
+        );
     }
 }

test/BobToken.t.sol

@@ -144,6 +144,26 @@ contract BobTokenTest is Test, EIP2470Test {
         bob.permit(user1, user2, 1 ether, expiry, v, r, s);
     }
 
+    function testPermitFailsAfterHardFork() public {
+        vm.prank(user1);
+        bob.mint(user1, 1 ether);
+
+        uint256 expiry = block.timestamp + 1 days;
+        (uint8 v, bytes32 r, bytes32 s) = _signPermit(pk1, user1, user2, 1 ether, 0, expiry);
+
+        bytes32 sep = bob.DOMAIN_SEPARATOR();
+
+        vm.chainId(1234);
+        assertTrue(sep != bob.DOMAIN_SEPARATOR());
+        vm.expectRevert("ERC20Permit: invalid ERC2612 signature");
+        bob.permit(user1, user2, 1 ether, expiry, v, r, s);
+
+        vm.chainId(31337);
+        assertTrue(sep == bob.DOMAIN_SEPARATOR());
+        bob.permit(user1, user2, 1 ether, expiry, v, r, s);
+        assertEq(bob.allowance(user1, user2), 1 ether);
+    }
+
     function testReceiveWithPermit() public {
         vm.prank(user1);
         bob.mint(user1, 1 ether);