Summary

The depositCollateral function currently invokes the standard ERC20 transferFrom() method to move collateral from the users to the DSCEngine contract. This may cause an execution failure when transferring assets whose contracts do not conform with the latest ERC20 specification.

Vulnerability Details

As per the ERC20 standard, all transfer methods should return a boolean value to indicate the status of the transfer. However, if the DSCEngine contract were to include assets that do not follow such standards (e.g. USDT) as collateral, the deposit would fail, because USDT does not return a bool value after the transfer has completed, regardless of whether it fails or succeeds.

Impact

Projects may not be able to include other reputable asset types, such as USDT as collateral for their forked DSCEngine. This is under the assumption that there is an intention for DSCEngine to include collaterals other than WETH and WBTC, which however, are not affected by this vulnerability.

Tools Used

Tenderly and Foundry.
DSCEngine contract is deployed to a forked Ethereum mainnet on Tenderly, configured with USDT as one of the collaterals.

Recommendations

Replace IERC20 transfer methods with OpenZeppelin's SafeERC20 transfers. https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/utils/SafeERC20.sol