sherlock-audit / 2023-03-teller-judging

8 stars 6 forks source link

ArbitraryExecution - ERC20 token transfer can fail #501

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ArbitraryExecution

high

ERC20 token transfer can fail

Summary

The withdrawCollateral function executes an unsafe transfer function on an ERC20 token. This does not take into account ERC20 tokens that return false instead of reverting on token transfer error. An example of this is USDT, it will return false instead of reverting on error.

Vulnerability Detail

Impact

Collateral could be lost and unable to be withdrawn

Code Snippet

        // Withdraw ERC20
        if (_collateral._collateralType == CollateralType.ERC20) {
            IERC20Upgradeable(_collateralAddress).transfer(
                _recipient,
                _collateral._amount
            );
        }

Tool used

Manual Review

Recommendation

Consider using the OpenZeppelin safeTransferFrom function when transferring ERC20 assets

Duplicate of #220