15,000 USDC
View results
Submission Details
Severity: medium
Valid

Wrong Calculation When Fee-on-transfer Token Is Used As Collateral

Summary

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.

Vulnerability Details

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.

Impact

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.

Tools Used

Manual Review

Recommendations

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:

function depositCollateral(address tokenCollateralAddress, uint256 amountCollateral)
public
moreThanZero(amountCollateral)
isAllowedToken(tokenCollateralAddress)
nonReentrant
{
uint256 valueBefore = IERC20(tokenCollateralAddress).balanceOf(address(this));
bool success = IERC20(tokenCollateralAddress).transferFrom(msg.sender, address(this), amountCollateral);
if (!success) {
revert DSCEngine__TransferFailed();
}
uint256 valueAfter = IERC20(tokenCollateralAddress).balanceOf(address(this));
uint256 receivedCollateral = valueAfter - valueBefore;
s_collateralDeposited[msg.sender][tokenCollateralAddress] += receivedCollateral;
emit CollateralDeposited(msg.sender, tokenCollateralAddress, receivedCollateral);
}

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.