Foundry DeFi Stablecoin CodeHawks Audit Contest
Cyfrin
DeFiFoundry
15,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Use safeTransfer and safeTransferFrom instead of transfer and transferFrom for ERC20 contracts

peanuts

Summary

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

Vulnerability Details

Some tokens (like USDT) don't correctly implement the EIP20 standard and their transfer/ transferFrom function return void instead of a success boolean. Calling these functions with the correct EIP20 function signatures will always revert.

function depositCollateral(address tokenCollateralAddress, uint256 amountCollateral)
public
moreThanZero(amountCollateral)
isAllowedToken(tokenCollateralAddress)
nonReentrant
{
s_collateralDeposited[msg.sender][tokenCollateralAddress] += amountCollateral;
emit CollateralDeposited(msg.sender, tokenCollateralAddress, amountCollateral);
bool success = IERC20(tokenCollateralAddress).transferFrom(msg.sender, address(this), amountCollateral);
if (!success) {
revert DSCEngine__TransferFailed();
}
}

Impact

Tokens that don't correctly implement the latest EIP20 spec, like USDT, will be unusable in the protocol as they revert the transaction because of the missing return value. Since this is a borrowing and lending platform, any ERC20 tokens should be applicable.

Tools Used

Manual Review

Recommendations

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.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.