Fee-on-transfer tokens behave differently from standard ERC20 tokens, as the amount received by the destination may not be the same as the amount sent by the sender due to transfer taxes or fees.
The impact of this vulnerability is significant, as it can lead to incorrect calculations of the user's collateral value in the protocol. This can result in the protocol considering the user's collateral to be more than its real value, potentially preventing the user from being liquidated when required or allowing the user to mint more tokens than they should, causing under-collateralization issues.
To demonstrate the impact, the text provides a proof of concept with an example involving USDT as collateral, which incurs a 1% transfer fee. The protocol does not correctly account for the fee, leading to discrepancies in collateral calculations and potential issues with liquidation and loss of collateral value over time.
The recommended mitigation steps involve updating the depositCollateral()
function to accurately calculate the amount received by the user, taking into account the balance before and after receiving the collateral.
The DSCEngine
contract assumes that all ERC20 contracts send all token amounts to the destination. However, fee-on-transfer tokens (for example those with transfer taxes) behave differently. The amount of tokens sent by the sender is not the same as the amount of tokens received by the destination. In the description of the project, it has been mentioned that:
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.
However, using transfer-on-fee tokens can cause problems when they are used in this protocol. In this case, protocol considers the user's collateral more than the real value. This problem can prevent the user from liquidation or can, even in the scenarios that the user must get liquidated.
As mentioned previously, protocol considers the user's collateral more than the real value. This problem can prevent the user from liquidation or can, even in the scenarios that the user must get liquidated.
Consider we have a DSCEngine
that supports USDT as the collateral, and USDT takes 1% of the sent amount. Assume that Alice sends $1,000 USDT to mint $500 DSC. If $1,000 is received by DSCEngine
, the health factor is more than MIN_HEALTH_FACTOR
and the user must not get liquidated. However, when the $1,000 USDT is sent; $10 is taken as the fee, and 990 USDT will be received by the protocol. In this case, the protocol can mint up to $495 DSC, and any values more than that can cause the health factor to be less than MIN_HEALTH_FACTOR
. So, the user must not be able to mint $500, and the user must get liquidated; however, the user mints $500, and since the protocol thinks the sent amount is $1,000, does not allow liquidation.
Also, consider the user wants to pay the $500 DSC and take back $1,000 USDT. In this case, the protocol sends $1,000, and the user receives $990 USDT. The important issue is that the protocol received $990; however, sent $1,000 and lost $10. This can lead to loss of the collateral in the progress of time, and the coin may become undercollateralized and lose its price.
Manual Review
For calculating the amount received by the user; you have to calculate the balance of the DSCEngine
before and after receiving the collateral and use the difference between those two values for the collateral amount.
For example, change the depositeCollateral()
function to the below:
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.