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

Use `safeTransfer` for token transfer

aak

Summary

If non-compliant ERC20 tokens are added in future as collateral (like USDT), then protocol may not be able to handle the transfers.

Vulnerability Details

The protocol use transfer and transferFrom to transfer collateral token. It also verifies that its return value is true:

bool success = IERC20(tokenCollateralAddress).transferFrom(msg.sender, address(this), amountCollateral);
if (!success) {
revert DSCEngine__TransferFailed();
}

For WETH and WBTC, if transfer doesn't revert, success is always true. Hence this check can be skipped to save gas. However, if we want to add other collateral tokens in future, it's recommended to use OpenZeppelin's SafeERC20 library which accommodates non-compliant ERC20 tokens. For example, USDT which doesn't return anything on a successful transfer.

Impact

Tools Used

Manual review.

Recommendations

Use OpenZeppelin's SafeERC20 library to handle ERC20 transfer. Add the following at the top of the contract:

using SafeERC20 for IERC20;

Now replace .transfer and .transferFrom with .safeTransfer and .safeTransferFrom. For instance:

+IERC20(tokenCollateralAddress).safeTransferFrom(msg.sender, address(this), amountCollateral);
-bool success = IERC20(tokenCollateralAddress).transferFrom(msg.sender, address(this), amountCollateral);
-if (!success) {
- revert DSCEngine__TransferFailed();
-}

Support

FAQs

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