code-423n4 / 2022-10-thegraph-findings

0 stars 0 forks source link

Multichain signature reuse risk when use the signature to grant allowance permission in GraphTokenUpgradeable.sol #147

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-10-thegraph/blob/309a188f7215fa42c745b136357702400f91b4ff/contracts/l2/token/GraphTokenUpgradeable.sol#L42 https://github.com/code-423n4/2022-10-thegraph/blob/309a188f7215fa42c745b136357702400f91b4ff/contracts/l2/token/GraphTokenUpgradeable.sol#L88 https://github.com/code-423n4/2022-10-thegraph/blob/309a188f7215fa42c745b136357702400f91b4ff/contracts/l2/token/GraphTokenUpgradeable.sol#L94

Vulnerability details

Impact

Detailed description of the impact of this finding.

The GraphTokenUpgradeable support the usage of offline signature to approve token spending.

However, there is Multichain signature reuse risk when user signing the signature, the attacker can take the user's signature, use to approve user's token spending in another blockchain because the chainId is not present in the signature field

   bytes32 private constant PERMIT_TYPEHASH =
        keccak256(
            "Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"
        );

the permit hash only use the owner, spender, permitted amount, nonce and deadline, but not the chainId

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

  1. A User Alice create a signature and sign the transaction to approve an account using his token in Blockchain network 1.

  2. The transaction go through. the Hacker detects the transaction, take the signature, and use to approve the token allowance in another blockchain network.

  3. The hacker drain user's fund in another network.

Tools Used

Manual Review

Recommended Mitigation Steps

We recommend the project add chainId to the type hash

     bytes32 private constant PERMIT_TYPEHASH =
        keccak256(
            "Permit(address owner,address spender, uint256 chainId, uint256 value,uint256 nonce,uint256 deadline)"
        );

later when we recover the signature, we also add chainId

uint256 chainId =  _getChainID();
bytes32 digest = keccak256(
            abi.encodePacked(
                "\x19\x01",
                DOMAIN_SEPARATOR,
                keccak256(
                    abi.encode(PERMIT_TYPEHASH, _owner, _spender, , chainId, _value, nonces[_owner], _deadline)
                )
            )
        );

to avoid the multichain signature reuse risk.

0xean commented 2 years ago

see the DOMAIN_SEPARATOR