tibthecat - Unsafe usage of ERC20 transfer

[sherlock-admin]

tibthecat

medium

Unsafe usage of ERC20 transfer

Summary

Some ERC20 transfers could fail without reverting, engendering loss of funds.

Vulnerability Detail

Some ERC20 tokens don't revert when transfer fails, they may return a boolean or do nothing (see USDT or ZRX tokens).
And ERC20 unsafe transfer function is used 2 times in the footium codebase.

In FootiumEscrow.sol:

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

And in FootiumPrizeDistributor.sol:

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

Impact

In FootiumPrizeDistributor.sol, if the transfer fails, without reverting, the contract state will assume that prize has been claimed, hence a loss of funds for the prize claimer.

In FootiumEscrow.sol, if the transfer fails witjout reverting, the user might think he has transfered the tokens whereas he has not which could engender some troubles.

Note: In these two cases, if the token used is the FootiumToken there is no risk, but FootiumPrizeDistributor.sol and FootiumEscrow.sol can manage any ERC20 token and it is not written in the documentation that FootiumToken will be the only token that might be used by these contracts.

Code Snippet

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

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

Tool used

Manual Review

Recommendation

Use OpenZeppelin's SafeERC20 library so that failed transfers would revert.

Duplicate of #86