0xPkhatri - Implement safeTransfer() instead of transfer()

[sherlock-admin]

0xPkhatri

medium

Implement safeTransfer() instead of transfer()

Summary

The ERC20.transfer() function returns a Boolean value indicating success, which must be checked for proper execution. Some tokens do not revert if the transfer fails but return false instead.

Vulnerability Detail

Certain non-compliant ERC20 tokens may return false from the transfer and transferFrom function calls, signaling that the transfer has failed. However, if the return value is not checked, the calling contract will not detect the failure, despite the EIP-20 specification requiring a return value check.

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

Impact

The protocol or users might experience losses.

Code Snippet

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

Tool used

Manual Review

Recommendation

It is recommended to use OpenZeppelin's SafeERC20 utility, which includes safeTransfer functions that handle the return value check and non-standard-compliant tokens.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/utils/SafeERC20.sol

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

}

Duplicate of #86