DittoETH

Summary

Several computations face precision-loss issues detected throughout the codebase.

Vulnerability Details

This report lists all detected computations that face precision-loss issues throughout the codebase. For more details, please refer to the Proof of Concept section below.

• High Severity

  • eth in LibOrders::addShort()

  • matchTotal.colUsed in LibOrders::matchHighestBid()

  • cRatio in LibShortRecord::getCollateralRatio()

  • cRatio in LibShortRecord::getCollateralRatioSpotPrice()

  • p.eth.mul(cr) in ShortOrdersFacet::createLimitShort() -- in p.eth and cr, there are the multiplication and division operations. In p.eth.mul(cr), there are 3 divisions before multiplications.

  • m.ethDebt in MarginCallPrimaryFacet::_setMarginCallStruct()

  • eth in OrdersFacet::cancelShort()

  • cRatio in MarketShutdownFacet::_getAssetCollateralRatio()

  • amtZeth in MarketShutdownFacet::redeemErc() -- there are 4 divisions before multiplications

• Medium Severity

  • ethAmount in BridgeRouterFacet::_ethConversion()

  • m.short.ercDebt in MarginCallPrimaryFacet::_performForcedBid()

• Medium/Low Severity

  • twapPriceInEther in LibOracle::baseOracleCircuitBreaker()

  • shares in LibOrders::increaseSharesOnMatch()

  • tithe in LibVault::updateYield() -- there are the multiplication and division operations in the zethTithePercent()

  • zethTreasuryReward in LibVault::updateYield()

  • fee in BridgeRouterFacet::withdraw() -- there are the multiplication and division operations in the withdrawalFee

  • fee in BridgeRouterFacet::unstakeEth() -- there are the multiplication and division operations in the bridge.unstakeFee()

  • dittoYieldShares in YieldFacet::_distributeYield()

  • dittoReward in YieldFacet::_claimYield()

  • userReward in YieldFacet::claimDittoMatchedReward()

  • colUsed in BidOrdersFacet::matchlowestSell() -- there are the multiplication and division operations in the LibOrders.convertCR()

Proof of Concept

To demonstrate, let's consider the computation of the cRatio in the LibShortRecord::getCollateralRatio().

As you can see, the computation has div() and mul(). If we drill down into their implementation, we will find the mulDiv() inside both. Thus, the computation of the cRatio comprises multiple divisions before multiplications, leading to the precision-loss issue.

Since the core functions throughout the Ditto protocol use the getCollateralRatio(), the loss of precision of the cRatio can cause massive damage to the protocol's users more than roughly estimable (Impact: HIGH).

• https://github.com/Cyfrin/2023-09-ditto/blob/a93b4276420a092913f43169a353a6198d3c21b9/contracts/libraries/LibShortRecord.sol#L27

• https://github.com/Cyfrin/2023-09-ditto/blob/a93b4276420a092913f43169a353a6198d3c21b9/contracts/libraries/PRBMathHelper.sol#L9

• https://github.com/Cyfrin/2023-09-ditto/blob/a93b4276420a092913f43169a353a6198d3c21b9/contracts/libraries/PRBMathHelper.sol#L13

Impact

The loss of precision in the computations listed in this report can cause massive damage to the protocol's users, more than roughly estimable (Impact: HIGH).

Tools Used

Manual Review

Recommendations

Improve the affected computations by taking multiplications before divisions to prevent truncation.