Foundry DeFi Stablecoin CodeHawks Audit Contest
Cyfrin
DeFiFoundry
15,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

User's healthFactor calculation is wrong if user deposit with a token that don't have 18 decimals

kose

Summary

getUsdValue function assumes that amount parameter it is taken have 18 decimals. Because DSCEngine is supposed to be able to work with other tokens which have different decimals (for example WBTC have 8 decimals) user's healthFactor calculation will be wrong and _revertIfHealthFactorIsBroken will always returns true if user deposits with tokens with less than 18 decimals. Although it is not common in practice, if user deposits with tokens with more than 18 decimals they will be able to mint more DSC than expected.

Vulnerability Details

getUsdValue function's amount parameter is coming from getAccountCollateralValue function's amount calculation. This amount calculation is taken from s_collateralDeposited[user][token]. Lets use WBTC in our example. If user deposits WBTC which have 8 decimals, this value will have 8 decimals. Hence calculation below will be done with 8 decimal amount:

function getUsdValue(address token, uint256 amount) public view returns (uint256) {
AggregatorV3Interface priceFeed = AggregatorV3Interface(s_priceFeeds[token]);
(, int256 price,,,) = priceFeed.staleCheckLatestRoundData();
// 1 ETH = $1000
// The returned value from CL will be 1000 * 1e8
return ((uint256(price) * ADDITIONAL_FEED_PRECISION) * amount) / PRECISION;

Let's analyze the return value's decimal:
uint256(price) = 8 decimals
ADDITIONAL_FEED_PRECISION = 10 decimals
amount = 8 decimals
PRECISION = 18 decimals
Hence result will be in 8 (8+10+8/18) decimals. Then this value will be used in _getAccountInformation as collateralValueInUsd, then will be used in _healthFactor as collateralValueInUsd. Finally _calculateHealthFactor function will use this value while calculating HealthFactor as shown:

function _calculateHealthFactor(uint256 totalDscMinted, uint256 collateralValueInUsd)
internal
pure
returns (uint256)
{
if (totalDscMinted == 0) return type(uint256).max;
uint256 collateralAdjustedForThreshold = (collateralValueInUsd * LIQUIDATION_THRESHOLD) / LIQUIDATION_PRECISION;
return (collateralAdjustedForThreshold * 1e18) / totalDscMinted;
}

This function will return at 8 decimals because:
collateralAdjustedForThreshold is in 8 decimals, and totalDscMinted is in 18 decimals.
This 8 decimal value will be used in _revertIfHealthFactorIsBroken as a userHealthFactor as shown below: (Because _healthFactor calls _calculateHealthFactor and returns its return value which is in 8 decimals)

function _revertIfHealthFactorIsBroken(address user) internal view {
uint256 userHealthFactor = _healthFactor(user);
if (userHealthFactor < MIN_HEALTH_FACTOR) {
revert DSCEngine__BreaksHealthFactor(userHealthFactor);
}
}

Because MIN_HEALTH_FACTOR is in 18 decimals:

uint256 private constant MIN_HEALTH_FACTOR = 1e18;

This function will always revert if only called with WBTC.
Also if user both uses WETH and WBTC, their WBTC collateral won't have any meaningful effect, hence it is again pointless to use WBTC in this case. This of course applies to all other tokens that have different decimal amount than 18.

Impact

The protocol user can not use any other tokens that are not 18 decimals properly, for example there is no way to mint DSC using WBTC. Although there is no loss of funds, since WBTC is protocol's one of main collateral and protocol also assumes to be used with any token that have chainlink priceFeed with USD counterparty, this issue creates a medium severity vulnerability.

Tools Used

Manual Review

Recommendations

In getUsdValue function, first check the decimal of token and then calculate accordingly.

Support

FAQs

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