depositCollateral’s internal accounting does not properly handle fee on transfer / rebase tokens

Summary

There is a wide variety of ERC20 compliant tokens and 2 of such types (fee on transfer, rebase tokens) are not well supported by the DSCEngine.

Vulnerability Detail

depositCollateral assumes that the amount of tokens transferred to the DSCEngine is always equal to amountCollateral. But this is not necessarily the case for fee on transfer tokens like USDT or rebase tokens like AMPL.

Impact

For fee on transfer tokens, DSCEngine’s internal accounting is will be incorrect over time can lead to an accumulation of bad debt since the engine receives less collateral than expected.

The vulnerability for rebase tokens is more substantial e.g. if I deposit 2000 tokens worth 10 dollars each (total collateral 20k USD) to mint X DSC and it rebased to 1000 tokens. The oracle should report a new price of 20 dollars each so the engine records that I have a total collateral value of 40k. I can then mint more DSC without depositing more collateral.

Tool used

Manual Review

Recommendation

Since the engine is created by an owner, the owner should not allow users to mint DSC with rebase tokens like AMPL. For fee on transfer tokens, you can simply track the USDT balance before and after and record the difference instead of trusting amountCollateral.