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

Use `safeTransfer` and `safeTransferFrom` for token transfers, else breaks integration with certain collateral

toshii

Summary

There are multiple cases in which transfer and transferFrom are used instead of safeTransfer and safeTransferFrom. As per specs, wETH and wBTC should be interchangeable with other collateral that is supported by Chainlink, such as USDT. However, on Ethereum, USDT transfers do not return a bool, meaning that all attempts to transfer this token will revert. This means tokens like USDT which don't return a bool on transfers cannot be used as collateral, breaking core functionality.

Vulnerability Details

In the depositCollateral function there is the following call:

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

This will revert for all tokens which don't return a bool on transfers, such as USDT on Ethereum. This is because success will not be true, even if the transfer succeeded.

Impact

Certain tokens will not be usable as collateral, breaking core functionality of this protocol.

Tools Used

Manual review

Recommendations

Replace all transfer and transferFrom calls with safeTransfer and safeTransferFrom.

Support

FAQs

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