Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
code423n4 opened this issue Jun 17, 2021 · 2 comments
Open

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

code423n4 opened this issue Jun 17, 2021 · 2 comments
Labels

Comments

@code423n4
Copy link
Contributor

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
Copy link
Collaborator

Splidge commented Jun 18, 2021

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
Copy link
Collaborator

Splidge commented Jun 21, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants