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

Precision loss when calculating the health factor

serialcoder

Summary

The calculation of the health factor in the _calculateHealthFactor() suffers from a rounding down issue, resulting in a small precision loss that can be improved.

Vulnerability Details

Division before multiplication can lead to rounding down issue since Solidity has no fixed-point numbers. Consider the calculation of the health factor in the _calculateHealthFactor(), the function does the division (by LIQUIDATION_PRECISION) before the multiplication (by 1e18). Hence, the computed result can suffer from the rounding down issue, resulting in a small precision loss.

function _calculateHealthFactor(uint256 totalDscMinted, uint256 collateralValueInUsd)
internal
pure
returns (uint256)
{
if (totalDscMinted == 0) return type(uint256).max;
@> uint256 collateralAdjustedForThreshold = (collateralValueInUsd * LIQUIDATION_THRESHOLD) / LIQUIDATION_PRECISION; //@audit Division (by LIQUIDATION_PRECISION) before multiplication
@> return (collateralAdjustedForThreshold * 1e18) / totalDscMinted; //@audit Multiplication (by 1e18) after division
}

  • Division (by LIQUIDATION_PRECISION) before multiplication: https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L330

  • Multiplication (by 1e18) after division: https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L331

Impact

The computed result of the _calculateHealthFactor() can suffer from the rounding down issue. However, the impact can be considerably low since the denominator, LIQUIDATION_PRECISION, is a constant of 100. Anyway, there can be a way to improve the calculation for better precision loss (see the Recommendations section).

Tools Used

Manual Review

Recommendations

I recommend improving the affected formula by taking multiplications before divisions to prevent truncation, as shown below.

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;
​
+ return (collateralValueInUsd * LIQUIDATION_THRESHOLD * 1e18) / (LIQUIDATION_PRECISION * totalDscMinted);
}

Support

FAQs

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