libratus - Not checking the return value of ERC20 transfers

[github-actions[bot]]

libratus

medium

Not checking the return value of ERC20 transfers

Summary

Not checking whether ERC20 transfer is successful may lead to unexpected behavior for certain types of tokens.

Vulnerability Detail

Some ERC20 implementations do not revert if tokens cannot be transferred, and instead return false. Not checking for this return value may lead to some incorrect behaviors.

For example, in UxdController._redeem function it may happen that user`s UXD tokens get burned but collateral tokens are not returned to the user. This part is shown in the snippet section below.

Impact

Users may lose funds if collateral's ERC20 implementation uses return value to indicate successful transfer

Code Snippet

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/core/UXDController.sol#L336-L339

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/integrations/perp/PerpDepository.sol#L220-L222

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/integrations/perp/PerpDepository.sol#L300-L301

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/integrations/perp/PerpDepository.sol#L518-L522

Tool used

Manual Review

Recommendation

Check for return value when using ERC20 transfer or transferFrom.

    let success = IERC20(redeemParams.assetToken).transfer(redeemParams.intermediary, amountOut);
    require(success, 'Token transfer failed');
    return amountOut;