If the collateral tokens that user own, have different decimals (not 18) user's health factor will be calculated incorrectly.
In DSCEngine
contract in order to user be able to redeem collateral, mint DSC, burn DSC and liquidation, thier health factor should be greater than the minimum and this check is done with help of _healthFactor
function.
_healthFactor
function use two values related to user for calculate the health factor, first one is totalDscMinted
and second one is collateralValueInUsd
.
But collateralValueInUsd
which is user's total collateral in USD, is calculated incorrectly. Lets see the problem:
this value is getting calculated in getAccountCollateralValue
function
As you can see it loop through all collateral tokens allowed in contract and for getting the price of each token in USD it call getUsdValue
function, But the problem is this function return the token price with the token's decimals and the returned value gets added to totalCollateralValueInUsd
variable, here is the getUsdValue
calculataion:
lets say price is 10$ with 18 decimals 10000000000000000000
and the amount of token passed is 4 with 8 decimals 400000000
. (10000000000000000000 * 400000000) / 1e18
returned value from this calculation is 4000000000
which is based of the token decimals not 18 decimals.
So by knowing this, getAccountCollateralValue
function adds the returned value from getUsdValue
to totalCollateralValueInUsd
variable which is wrong, adding different token values with different decimals will end up with incorrect value.
Imagine user has two collateral tokens: WETH
(18 decimals) and WBTC
(8 decimals)
getAccountCollateralValue
adds the balance of user in both assets in USD together, (we know that returned USD value of WETH has 18 decimals and returned USD value of WBTC has 8) if amount WETH is 5 tokens and WBTC 2 sum of them will be 5000000000200000000
Here are the problems that this vulnerability can make (all of this happens because collateralValueInUsd
will be lower than expected and health factor will be reduced too much):
Redeeming : user should have alots of collateral in order to redeem small value othewise it reverts.
Minting : user should have alots of collateral in order to mint small amount of tokens othewise it reverts.
Burning : Again user should have alots of collateral in order to burn small amount of tokens othewise it reverts.
Liquidation : user will get liquidated and they loose their collateral.
View function : if any important offchain operations rely on health factor or total collateral of user, this will break alots of things
Manual Analysis
Since you compare the returned value from _calculateHealthFactor
with the MIN_HEALTH_FACTOR = 1e18
, the collateralValueInUsd should be based of 18 decimals.
So in getAccountCollateralValue
before passing the amount to getUsdValue
and add it with total amount, you need to scale amount's precision to 18 if token's decimals is not 18, it should like this:
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.