sherlock-audit/2023-02-gmx-judging

joestakey - Incorrect funding amount due to precision loss in `getNextFundingAmountPerSize()` for markets with low open interest

Closed this issue · 0 comments

joestakey

medium

Incorrect funding amount due to precision loss in getNextFundingAmountPerSize() for markets with low open interest

Summary

Rounding happens in getNextFundingAmountPerSize() due to division before mulitplication, leading to a loss of precision impacting the funding amount.

Vulnerability Detail

cache.fundingUsd is computed by first dividing cache.sizeOfLargerSide by Precision.FLOAT_PRECISION == 1e30.
The issue is that cache.sizeOfLargerSide also has 30 decimals, meaning that if the open interests are low enough, the rounding will be significant.
For instance: cache.oi.longOpenInterest = 1.99999e30 -> cache.sizeOfLargerSide / Precision.FLOAT_PRECISION = 1.

File: contracts/market/MarketUtils.sol
941:     cache.sizeOfLargerSide = cache.oi.longOpenInterest > cache.oi.shortOpenInterest ? cache.oi.longOpenInterest : cache.oi.shortOpenInterest;
942:         result.fundingFactorPerSecond = getFundingFactorPerSecond(
943:             dataStore,
944:             market.marketToken,
945:             cache.diffUsd,
946:             cache.totalOpenInterest
947:         );
948:         cache.fundingUsd = (cache.sizeOfLargerSide / Precision.FLOAT_PRECISION) * cache.durationInSeconds * result.fundingFactorPerSecond;//@audit precision loss 

Impact

Precision loss results in a lower funding amount, which means the position fees computed in PositionPricingUtils.getPositionFees() will be incorrect.

Code Snippet

https://github.com/gmx-io/gmx-synthetics/blob/7be3ef2d119d9e84473e1a49f346bcdc06fd57a3/contracts/market/MarketUtils.sol#L941-L948

Tool used

Manual Review

Recommendation

Refactor so that division happens after multiplication.
Overflow could technically occur for very large open interests, but this is expected as per the comments in getTotalBorrowing().

+948:         cache.fundingUsd = cache.sizeOfLargerSide * cache.durationInSeconds * result.fundingFactorPerSecond  / Precision.FLOAT_PRECISION;
-948:         cache.fundingUsd = (cache.sizeOfLargerSide / Precision.FLOAT_PRECISION) * cache.durationInSeconds * result.fundingFactorPerSecond;

Duplicate of #113