Summary

USDT cannot be deposited or redeemed as collateral in DSCEngine.

Vulnerability Details

Certain tokens, most notably USDT, do not return a boolean in their implementations of transfer(), transferFrom() and approve(). DSCEngine makes external calls to the collateral asset's transfer() and transferFrom() in the deposit and redeem flows. If the collateral is one of these exceptional tokens, then Solidity will panic and revert the execution, expecting return data when there is none.

The effect is that DSCEngine will not work with USDT (and similar) as collateral. This limitation runs contrary to the design philosophy of DSCEngine (maximally compatible with any assets):

The system is meant to be such that someone could fork this codebase, swap out WETH & WBTC for any basket of assets they like, and the code would work the same.

More about this vulnerability: https://medium.com/coinmonks/missing-return-value-bug-at-least-130-tokens-affected-d67bf08521ca

Impact

Medium.

The incompatibility may not be immediately apparent to borrowers. In a deployment where USDT is part of the basket of available collateral assets, users may erroneously expect that USDT can be used as collateral, which presents a skewed picture of the risk profile of their position.

Tools Used

Manual Review

Recommendations

Use OpenZeppelin's SafeERC20 Library to safely handle this and other exceptional cases with ERC20 interactions.