sherlock-audit / 2023-12-dodo-gsp-judging

6 stars 5 forks source link

hash - cached domainSeperator allows for replay in case of chain forks #126

Closed sherlock-admin closed 10 months ago

sherlock-admin commented 10 months ago

hash

medium

cached domainSeperator allows for replay in case of chain forks

Summary

Fixed chainId can cause EIP-712 permit replay in case of chain fork

Vulnerability Detail

DOMAIN_SEPARATOR is prepared in the init function with the current chainId. The permit function which validates the signature reuses this DOMAIN_SEPARATOR without validating whether the chainId has changed due to a fork.

    function permit(
        .....
    ) external {
        ......

        bytes32 digest =
            keccak256(
                abi.encodePacked(
                    "\x19\x01",
                    DOMAIN_SEPARATOR,
                    .....
                    )

Impact

Permit signatures on can be replayed across chains in case of a chain fork

Code Snippet

https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/af43d39f6a89e5084843e196fc0185abffe6304d/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPVault.sol#L311-L335

Tool used

Manual Review

Recommendation

Check for chainId and compute new DOMAIN_SEPARATOR in case it has changed (can refer to Openzeppelin implementation)

Duplicate of #113