EIP-02M: Improper Domain Separator Retrieval

[cliffhall]

EIP-02M: Improper Domain Separator Retrieval

Type	Severity	Location
Language Specific		EIP712Lib.sol:L67-L72

Description:

The EIP712Lib contract contains functions that dynamically construct the currently valid domain separator (i.e. domainSeparator) yet when getDomainSeparator is invoked the last value stored to domainSeparator is retrieved instead. As evidenced in the ConfigHandlerFacet contract that initializes this value, the domainSeparator is assigned to once thus permitting cross-chain replay attacks to occur, a trait especially relevant with the upcoming Ethereum hard fork.

Impact:

As the domainSeparator currently remains the same regardless of whether the blockchain the contract was deployed in forked, it is possible for a signed EIP-712 payload that is submitted on the original chain to be replayed on the forked chain improperly.

Example:

/**
 * @notice Get the domain separator.
 */
function getDomainSeparator() private view returns (bytes32) {
    return ProtocolLib.protocolMetaTxInfo().domainSeparator;
}

Recommendation:

We advise a caching system to be used instead similarly to the draft EIP-712 implementation by OpenZeppelin whereby the domain separator is re-calculated on a need-to basis by comparing the current block.chainid and the one that the original domainSeparator was calculated in, allowing the separator to be re-calculated in case the chain IDs do not match (i.e. due to the PoW chain of Ethereum being used instead of the PoS one).