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

claimAndDelegate in ERC20 Token airdrop is subject to DOS(denial of service) #181

Closed
c4-bot-1 opened this issue Mar 25, 2024 · 4 comments
Closed
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-201 grade-b QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_44_group AI based duplicate group recommendation

Comments

@c4-bot-1
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2024-03-taiko/blob/f58384f44dbf4c6535264a472322322705133b11/packages/protocol/contracts/team/airdrop/ERC20Airdrop.sol#L50

Vulnerability details

claimAndDelegate in ERC20 Token airdrop is subject to DOS(denial of service)

Line of code

  1. https://github.com/code-423n4/2024-03-taiko/blob/f58384f44dbf4c6535264a472322322705133b11/packages/protocol/contracts/team/airdrop/ERC20Airdrop.sol#L50
  2. https://github.com/OpenZeppelin/openzeppelin-contracts/blob/0c18fac08a10a53d651145d6447d607db1fd68b7/contracts/governance/utils/Votes.sol#L142

Proof of concept

Line of code

  function claimAndDelegate(
        address user,
        uint256 amount,
        bytes32[] calldata proof,
        bytes calldata delegationData
    )
        external
        nonReentrant
    {
        // Check if this can be claimed
        _verifyClaim(abi.encode(user, amount), proof);

        // Transfer the tokens
        IERC20(token).transferFrom(vault, user, amount);

        // Delegate the voting power to delegatee.
        // Note that the signature (v,r,s) may not correspond to the user address,
        // but since the data is provided by Taiko backend, it's not an issue even if
        // client can change the data to call delegateBySig for another user.
        (address delegatee, uint256 nonce, uint256 expiry, uint8 v, bytes32 r, bytes32 s) =
            abi.decode(delegationData, (address, uint256, uint256, uint8, bytes32, bytes32));

        // @audit
        // deinal of service?
        IVotes(token).delegateBySig(delegatee, nonce, expiry, v, r, s);
    }

The function claimAndDelegate let user claim the token then delegate the token via signature

        IVotes(token).delegateBySig(delegatee, nonce, expiry, v, r, s);

Impact

but malicious user can extract the signature and trigger delegateBySig directly first.

Refer to Votes.sol in OpenZeppelin.
Line of code

function delegateBySig(
        address delegatee,
        uint256 nonce,
        uint256 expiry,
        uint8 v,
        bytes32 r,
        bytes32 s
    ) public virtual {
        if (block.timestamp > expiry) {
            revert VotesExpiredSignature(expiry);
        }
        address signer = ECDSA.recover(
            _hashTypedDataV4(keccak256(abi.encode(DELEGATION_TYPEHASH, delegatee, nonce, expiry))),
            v,
            r,
            s
        );
        _useCheckedNonce(signer, nonce);
        _delegate(signer, delegatee);
    }

Then the signer's nonce is consumed and when the transaction claimAndDelegate executes,

the transaction revert because the same signature is already consumed,

this would grief and block user from executing the transaction claimAndDelegate

Recommendation

implement a mapping that tracks nonces specifically for the claimAndDelegate process,

and validates them before the delegateBySig function is called

Assessed type

Token-Transfer

@c4-bot-1 c4-bot-1 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Mar 25, 2024
c4-bot-2 added a commit that referenced this issue Mar 25, 2024
@c4-bot-12 c4-bot-12 added the 🤖_44_group AI based duplicate group recommendation label Mar 27, 2024
@c4-pre-sort
Copy link

minhquanym marked the issue as duplicate of #201

@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Apr 9, 2024
@c4-judge
Copy link
Contributor

c4-judge commented Apr 9, 2024

0xean changed the severity to QA (Quality Assurance)

@c4-judge c4-judge added grade-c unsatisfactory does not satisfy C4 submission criteria; not eligible for awards labels Apr 10, 2024
@c4-judge
Copy link
Contributor

0xean marked the issue as grade-c

@c4-judge c4-judge removed the grade-c label Apr 10, 2024
@c4-judge
Copy link
Contributor

0xean marked the issue as grade-b

@c4-judge c4-judge added grade-b and removed unsatisfactory does not satisfy C4 submission criteria; not eligible for awards labels Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-201 grade-b QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_44_group AI based duplicate group recommendation
Projects
None yet
Development

No branches or pull requests

4 participants