Closed code423n4 closed 1 year ago
Dup of #277
downgrading to match dupe of #277
M seems correct severity based on external requirements (fork) and the graph wanting to support its token on both networks.
Should this be closed as it is a dupe of #277?
Lines of code
https://github.com/code-423n4/2022-10-thegraph/blob/309a188f7215fa42c745b136357702400f91b4ff/contracts/l2/token/GraphTokenUpgradeable.sol#L86
Vulnerability details
Impact
DOMAIN_SEPARATOR
is not refreshed if the network changed.Proof of Concept
During a network fork, the chain id will change, for that reason the code should check if the chain id is the same as the stored one, otherwise, it will take the stored
DOMAIN_SEPARATOR
instead of use the new one. This will deal in a signature replay use, because in the two networks theDOMAIN_SEPARATOR
will be the same.Poc:
Note: that the contract is updatable, it will help to fix the problem (after the hack), not before this problem was known, and it's not know because it hasn't been fixed before.
Recommended Mitigation Steps