search

code-423n4 / 2021-06-realitycards-findings

3 stars 2 forks source link

The `domainSeperator` is not recalculated after a hard fork happens #166

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

shw

Vulnerability details

Impact

The variable domainSeperator in EIP712Base is cached in the contract storage and will not change after the contract is initialized. However, if a hard fork happens after the contract deployment, the domainSeperator would become invalid on one of the forked chains due to the block.chainid has changed.

Proof of Concept

Referenced code: EIP712Base.sol#L25-L44

Recommended Mitigation Steps

Consider using the implementation from OpenZeppelin, which recalculates the domain separator if the current block.chainid is not the cached chain ID.

Splidge commented 3 years ago

The OpenZeppelin implementation can't be used because of the contracts uses a proxy pattern and so can't use the constructor in the OpenZeppelin version. Instead I have taken the same method OpenZeppelin use to get a new domainSeperator, here

Splidge commented 3 years ago

I've noticed that there is an upgradable version of the OpenZeppelin metaTx contracts. I'll try and work on using them instead of the fix used above.