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

`_healthFactor` calculation is broken when assets with less than 18 decimals are used

Summary

The function _revertIfHealthFactorIsBroken calculates the _healthFactor of an user and then compares it with the MIN_HEALTH_FACTOR, reverting if it less than it, which will be 100% of the time when the assets used have lower decimals places.

Vulnerability Details

Let's take the example of WBTC which is one of the assets used in the protocol. WBTC has 8 decimals so if an user deposits WBTC as the collateral, getAccountCollateralValue which calls getUsdValue to calculate USD value of the collateral, will return a 1e8 number, as can be seen per the calculation https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L366
This value will then be passed into _calculateHealthFactor which will calculate the health factor, but in the case of WBTC the calculation would return a 1e8 number, since DSC inherits from ERC20Burnable and has 18 decimals https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L331
The health factor would then be passed into the if statement, which would always be true since you will compare a 1e8 number with MIN_HEALTH_FACTOR which is 1e18 and revert. Consider also that this can be used also in the liquidation process to liquidate someone even though they are not undercollateralized.

Impact

The impact is a people can be easily liquidated in those cases, even if they are well collateralized

Tools Used

Manual review

Recommendations

Try to do the calculations bringing every asset to the same decimal place, so accounting errors like this would not occur.

Support

FAQs

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