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

Not using SafeERC20 transfer wrapper could potentially cause issue

said

Summary

Some major tokens went live before ERC20 was finalized, resulting in a discrepancy whether the transfer functions should (A) return a boolean or (B) revert/fail on error. The current best practice is that they should revert, but return “true” on success. However, not every token claiming ERC20-compatibility is doing this — some only return true/false; some revert, but do not return anything on success.

This protocol doesn't use SafeERC20 transfer wrapper, could potentially cause issue when interacting with non standard ERC20.

Vulnerability Details

depositCollateral and _redeemCollateral not use ERC20 safeTransfer wrapper :

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L282-L291

function _redeemCollateral(address from, address to, address tokenCollateralAddress, uint256 amountCollateral)
private
{
s_collateralDeposited[from][tokenCollateralAddress] -= amountCollateral;
emit CollateralRedeemed(from, to, tokenCollateralAddress, amountCollateral);
bool success = IERC20(tokenCollateralAddress).transfer(to, amountCollateral);
if (!success) {
revert DSCEngine__TransferFailed();
}
}

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L149-L161

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

This can cause issue for some token that doesn't support standard ERC20, potentially lock the token on the contract.

Tools Used

Manual review

Recommendations

Use OpenZeppelin’s SafeERC20

Support

FAQs

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