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

Using transfer insted of safeTransfer

mahyar

Summary

Users may not be able to deposit collateral to engine because of the check for returned bool from transfer function.

Vulnerability Details

In DSCEngine contract users can deposit allowed tokens by protocol and since owner can set the allowed tokens to any tokens that has price feed there is possiblity the token they set not be ERC20-compliant.

not ERC20-compliant means tokens may not return a boolean when calling transfer or approve functions, so the check for boolean in the depositCollateral will revert and user will not be able to deposit that token as collateral

Impact

user calls depositCollateral function with passing not ERC20-compliant token address as collateral and the call will be reverted.

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();
}
}

Tools Used

Manual review

Recommendations

For fixing this problem you need to use SafeERC20 from OpenZeppelin.
(You also use the transfer for collateral tokens in _redeemCollateral function but if you don't fix this too users will not be able redeem)

Support

FAQs

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