Skip to content

Latest commit

 

History

History
44 lines (34 loc) · 1.81 KB

009.md

File metadata and controls

44 lines (34 loc) · 1.81 KB

Bauer

medium

Use safeTransferFrom() instead of transferFrom()

Summary

The ERC20.transfer() and ERC20.transferFrom() functions return a boolean value indicating success. This parameter needs to be checked for success. Some tokens do not revert if the transfer failed but return false instead.

Vulnerability Detail

Some ERC20 tokens that are not compliant with the specification could return false from the transfer and transferFrom functions call to indicate that the transfer fails, but the calling contract would not notice the failure if the return value is not checked.The EIP-20 specification requires to check the return value.

 if (collateralInfo._collateralType == CollateralType.ERC20) {
            IERC20Upgradeable(collateralInfo._collateralAddress).transferFrom(
                borrower,
                address(this),
                collateralInfo._amount
            );
            IERC20Upgradeable(collateralInfo._collateralAddress).approve(
                escrowAddress,
                collateralInfo._amount
            );
            collateralEscrow.depositAsset(
                CollateralType.ERC20,
                collateralInfo._collateralAddress,
                collateralInfo._amount,
                0
            );

Impact

The protocol or uses may suffer losses

Code Snippet

https://github.com/sherlock-audit/2023-03-teller/blob/main/teller-protocol-v2/packages/contracts/contracts/CollateralManager.sol#L327 https://github.com/sherlock-audit/2023-03-teller/blob/main/teller-protocol-v2/packages/contracts/contracts/escrow/CollateralEscrowV1.sol#L166

Tool used

Manual Review

Recommendation

Recommend using OpenZeppelin's SafeERC20 versions with the safeTransfer and safeTransferFrom functions that handle the return value check as well as non-standard-compliant tokens.