Foundry DeFi Stablecoin CodeHawks Audit Contest

Summary

It is specified in the description of the project that the assets used for this contract, WETH and WBTC, should be able to be changed with any other basket of assets and the protocol would work the same, which is not the case for some assets.

Vulnerability Details

The contract calls transferFrom in the depositCollateral and checks the return value of the call and if the return value is not true the call would revert, as can be seen here https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L157-L159 . The problem relies in the fact that some ERC20 like USDT doesn't return a boolean on transfer or transferFrom which would make every check fail.

Impact

This is a medium risk because the logic of the protocol is not working as expected with any basket of assets

Tools Used

Manual review

Recommendations

Consider implementing safeERC20 from OpenZeppeling or Solmate, which would solve the problems when working with assets that don't return booleans on transfer or transferFrom