Bauchibred - Users could have their price stuck in the PrizeDistributor contract

[sherlock-admin2]

Bauchibred

medium

Users could have their price stuck in the PrizeDistributor contract

Summary

Users could have their price stuck in the Distributor contract.

Vulnerability Detail

Take a look at FootiumPrizeDistributor.sol#L109-L145

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

        // Checks the Merkle proof, which is constructed as a hash of
        // the token address, the receiver address and the amount
            //@audit
        if (
            !MerkleProofUpgradeable.verify(
                _proof,
                erc20MerkleRoot,
                keccak256(abi.encode(_token, _to, _amount))
            )
        ) {
            revert InvalidERC20MerkleProof();
        }

        // The amount to be claimed is the difference between the total amount
        // and the amount already claimed.
        // This means that the amount passed in the function call is the total
        // amount that was ever earned by the user.
        uint256 value = _amount - totalERC20Claimed[_token][_to];

        // If there is still an amount to be claimed, transfer the tokens
        if (value > 0) {
            totalERC20Claimed[_token][_to] += value;
            _token.safeTransfer(_to, value);
        }

        emit ClaimERC20(_token, _to, value);
    }

As seen, this function is used by to to claim their prize tokens from the distributor contract.

Now from contest's readMe we can see that protocol is expected to work with "ANY" token, which would be a problem in regards to claiming the price in some cases.

To explain a bit further, it's no news that some ERC20 tokens exist with multiple entry points (also known as double entry tokens or two address tokens), now due to a reported bug case for one of the popular token that implements this feature, namely TUSD, the protocol curbed access from one of it's addresses. In addition, it is not unrealistic to expect that an upgradeable collateral token like USDT could become a double-entrypoint token in the future, so this must also be considered.

How this now affects Footium is that, if the integrated asset blocks the access of its logic from one of it's addresses (in this case the token address that's been used to create these merkle proofs on Footium), then even if verifying the proofs would work, attempting to then claim these prices would not work cause, the right address that was whitelisted is not supported by the underlying token itself and an attempt to withdraw would revert.

Impact

Core functionality is broken as no user would be able to claim proofs which is a DOS for all users who have prices in this token, also this DOS could as well last over 1 year, as taking into account the real life TUSD case it's been over 22 months since the patch was applied which blocked off support for one of it's addresses.

A subtle loss of US$ value could also be attached to this case as users might want to sell off their tokens but they can't access them and the price drops.

Now quoting Sherlock docs this should be a valid H/M as DOS is indefinite and there is a potential loss in US$ value.

Could Denial-of-Service (DOS), griefing, or locking of contracts count as a Medium (or High) issue?
It would not count if the DOS, etc. lasts a known, finite amount of time <1 year. If it will result in funds being inaccessible for >=1 year, then it would count as a loss of funds and be eligible for a Medium or High designation.

Code Snippet

FootiumPrizeDistributor.sol#L109-L145

Tool used

Manual Review

Recommendation

Since all ERC20s are going to be supported then do not rely on the token address in the accounting, alternatively have a whitelist of accepted tokens and these sets of tokens shouldn't be supported.

[sherlock-admin2]

2 comment(s) were left on this issue during the judging contest.

darkart commented:

The contract has receive/payable so there is no problem there

EV_om commented:

Not a duplicate of #28. Unrealistic assumptions and easily fixable by updating the Merkle root anyway.

[nevillehuang]

Based on the following, I believe this issue is invalid

1. This is purely speculation on USDT/other ERC20s being a potential double-entry point token in the future.
2. Future issues are not accepted based on sherlock rules unless explicitly mentioned in the DOCs/READ.ME.
3. Contracts are not expected to interact with tokens of non-standard behavior

Do you expect to use any of the following tokens with non-standard behaviour with the smart contracts?
None