tsvetanovv - Unsafe usage of ERC20 transfer()

[sherlock-admin]

tsvetanovv

medium

Unsafe usage of ERC20 transfer()

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.

Vulnerability Detail

Tokens that don't perform the transfer and return false are still counted as a correct transfer and tokens that don't correctly implement the latest EIP20 spec will be unusable in the protocol as they revert the transaction because of the missing return value.

Impact

Using unsafe ERC20 methods can revert the transaction for certain tokens.

Code Snippet

FootiumEscrow.sol
https://github.com/sherlock-audit/2023-04-footium/blob/main/footium-eth-shareable/contracts/FootiumEscrow.sol#L98-L111

function transferERC20(
        IERC20 erc20Contract,
        address to,
        uint256 amount
    ) external onlyClubOwner {
        erc20Contract.transfer(to, amount);
    }

FootiumPrizeDistributor.sol
https://github.com/sherlock-audit/2023-04-footium/blob/main/footium-eth-shareable/contracts/FootiumPrizeDistributor.sol#L97-L134

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

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