Skip to content
This repository has been archived by the owner on Nov 26, 2023. It is now read-only.

PokemonAuditSimulator - The transfer of ERC20 prizes may fail without reverting, resulting in the funds becoming locked #386

Closed
sherlock-admin opened this issue May 5, 2023 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented May 5, 2023

PokemonAuditSimulator

medium

The transfer of ERC20 prizes may fail without reverting, resulting in the funds becoming locked

Summary

Some ERC20 tokens do not revert when a transaction fails, resulting in users' funds being locked and inaccessible to them.

Vulnerability Detail

The claimERC20Prize() function can have unpredictable behavior or cause fund loss because some ERC20 tokens do not revert or return 0 upon transfer failure.

The claimERC20Prize() function does not include a check to ensure that the token transfer was successful. This means that if a transfer fails, the ERC20 tokens will still be marked as claimed from the PrizeDistributor contract, even though the user did not receive them.
For more detailed information, please refer to the following material: https://github.com/d-xo/weird-erc20#no-revert-on-failure.

Impact

This causes the funds of user to be locked and made inaccessible to them.

Code Snippet

[FootiumPrizeDistributor/L106-L134]

FootiumPrizeDistributor.sol

function claimERC20Prize(
        address _to,
        IERC20Upgradeable _token,
        uint256 _amount,
        bytes32[] calldata _proof
    ) external whenNotPaused nonReentrant {
        if (_to != msg.sender) {
            revert InvalidAccount();
        }

        if (
            !MerkleProofUpgradeable.verify(
                _proof,
                erc20MerkleRoot,
                keccak256(abi.encodePacked(_token, _to, _amount))
            )
        ) {
            revert InvalidERC20MerkleProof();
        }

        uint256 value = _amount - totalERC20Claimed[_token][_to];

        if (value > 0) {
            totalERC20Claimed[_token][_to] += value;

	    // @audit The following line can fail without reverting | will lead to locked funds!
            _token.transfer(_to, value);
        }

        emit ClaimERC20(_token, _to, value);
    }

Tool used

Manual Review

Recommendation

Consider modifying the specified line to mitigate the potential vulnerability. [FootiumPrizeDistributor/L130]
to:

require(_token.transfer(_to, value), "Token transfer failed.");

Implementing this change will effectively eliminate the potential vulnerability.

Duplicate of #86

@github-actions github-actions bot added Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels May 10, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label May 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

1 participant