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

Different token decimals not accounted for

Summary

Different token decimals are not accounted for, and can cause users to mint less tokens than they should be able to.

Vulnerability Details

In the getAccountCollateralValue, the amount of collateral an account has are added together:

totalCollateralValueInUsd += getUsdValue(token, amount);

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L356

The issue is that different tokens can have different decimals and introduce an error from the amount variable. Just like weth and wbtc have different decimals. weth has 18 decimals while wbtc has 8 decimals.

Consider this scenario:

For the sake of simplicity, let's assume the USD value of weth and wbtc are $2 each.

  • Bob deposits 1 weth (1e18): The USD value is $2

  • Bob also deposits 1 wbtc (1e8): The USD value is also $2

  • The total collateral value is: $4.

Since the liquidation threshold is 50, Bob should be able to mint 2 DSC tokens (2e18).

However, when Bob tries to mint 2 DSC tokens, the revertIfHealthFactorIsBroken fails. The healthFactor is less than the MINHEALTHFACTOR which is: 1e18.
Examining the getAccountCollateralValue function, the totalCollateralValueInUsd adds the USD value of the collaterals deposited.

totalCollateralValueInUsd += getUsdValue(token, amount);

The issue is that the getUsdValue function returns:

return ((uint256(price) * ADDITIONAL_FEED_PRECISION) * amount) / PRECISION;

From our example, while calculating the USD Value for weth, we get: (2e8)*(1e10)*(1e18)/(1e18). Which is: 2e18.
But while calculating the USD Value for wbtc, we get: (2e8)*(1e10)*(1e8)/(1e18). Which is 2e8.
Therefore, the totalCollateralValueInUsd would be 2000000000200000000, other than 4000000000000000000.

This in turn affects the health factor, which would be 500000000050000000, other than 1000000000000000000.

Running 1 test for test/unit/DSCEngineTestPOC.t.sol:DSCEngineTestPOC
[PASS] testRevertsIfMintAmountBreaksHealthFactorPOC() (gas: 250551)
Logs:
weth Price is $2: 200000000
wbth Price is $2: 200000000
Bob deposits 1 weth which is equivalent to $2
Bob deposits 1 wbtc which is equivalent to $2
Bob tries to mint 2 DSC tokens
weth USD value: 2000000000000000000
wbtc USD value: 200000000
Total Collateral USD value: 2000000000200000000
Expected Health Factor: 500000000050000000

Impact

Users would mint less tokens which in turn translates to loss of funds

Tools Used

Foundry

Recommendations

Consider checking the decimals and normalizing it before add the collateral values together.

Support

FAQs

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