#15 Return value of `transfer()` not checked (consider using safeTransfer)

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPool.sol#L175

https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPoolManager.sol#L40

Summary

Return values of transfer() not checked, beside that even after checking the return value, some tokens may never return value (like USDT and others).

Vulnerability Details

Not all ERC20 implementations revert() when there's a failure in transfer(). The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually transfer anything:

File: contracts/LiquidationPool.sol

175:                     IERC20(_token.addr).transfer(msg.sender, _rewardAmount);

Github: [175]

File: contracts/LiquidationPoolManager.sol

40:         eurosToken.transfer(protocol, eurosToken.balanceOf(address(this)));

Github: [40]

Impact

It may cause a situation where the protocol thinks the funds are transferred but actually not, and then it will cause a loss of funds.

Tools Used

Manual Review

Recommendations

Consider checking the return value of transfer if you are sure that the all tokens have a return value, otherwise consider using SafeERC20#safeTransfer.