0xAsen - Unsafe ERC20.transfer() - unchecked return values

[sherlock-admin]

0xAsen

medium

Unsafe ERC20.transfer() - unchecked return values

Summary

The ERC20.transfer() and ERC20.transferFrom() functions return a boolean value indicating success. This parameter needs to be checked for success.

Some tokens do not revert if the transfer failed but return false instead. (e.g. ZRX)

Vulnerability Detail

The unsafe .transfer() function is used in the claimERC20Prize function in the FootiumPrizeDistributor.sol contract.

    function claimERC20Prize(/*parameters*/) 
external whenNotPaused nonReentrant {
     //validations
        
        uint256 value = _amount - totalERC20Claimed[_token][_to];

        if (value > 0) {
            totalERC20Claimed[_token][_to] += value;
            _token.transfer(_to, value);
        }

        emit ClaimERC20(_token, _to, value);
    }

Implemented that way, there's a risk of a silent transfer failure which would lead to the internal balance of totalERC20Claimed being updated as the user supposedly received his tokens while in fact, he didn't receive anything.

This will lead to his tokens being stuck in the contract.

Impact

Could lead to stuck user funds in the contract, if a reward token that doesn't revert on failure but returns false instead is used.

Code Snippet

https://github.com/sherlock-audit/2023-04-footium/blob/11736f3f7f7efa88cb99ee98b04b85a46621347c/footium-eth-shareable/contracts/FootiumPrizeDistributor.sol#L106

https://github.com/sherlock-audit/2023-04-footium/blob/11736f3f7f7efa88cb99ee98b04b85a46621347c/footium-eth-shareable/contracts/FootiumEscrow.sol#L110

Tool used

Manual Review

Recommendation

Recommend using OpenZeppelin's SafeERC20 versions with the safeTransfer and safeTransferFrom functions that handle the return value check as well as non-standard-compliant tokens.

Duplicate of #86