DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: medium
Invalid

Miss calculation of the pending funding fee per unit value to accumulate in `PerpMarket::getPendingFundingFeePerUnit` function, resulting in a loss of precision in the number returned.

Relevant GitHub Links

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/leaves/PerpMarket.sol#L252

Summary

In the PerpMarket::getPendingFundingFeePerUnit function, where we calculate the pending funding fee per unit value to accumulate, we are called upon to perform multiplication operations and a division operation. However, division in solidity is rounded down to zero, which causes losses, and these losses are accentuated when multiplied.

Vulnerability Details

In Solidity, integer division naturally rounds down towards zero.So when we have an operation that includes multiplication and division, doing the division first makes us less accurate than doing the multiplication first.

function getPendingFundingFeePerUnit(
Data storage self,
SD59x18 fundingRate,
UD60x18 markPriceX18
)
internal
view
returns (SD59x18)
{
@> SD59x18 avgFundingRate = unary(sd59x18(self.lastFundingRate).add(fundingRate)).div(sd59x18Convert(2));
@> return avgFundingRate.mul(getProportionalElapsedSinceLastFunding(self).intoSD59x18()).mul(
markPriceX18.intoSD59x18()
);
}

Impact

Loss of precision in calculating of `the pending funding fee per unit value to accumulate` in the `PerpMarket::getPendingFundingFeePerUnit` function; which also leads to a loss of precision in the calculation of `the next funding fee per unit value` in function `PerpMarket::getNextFundingFeePerUnit` because the two functions are linked.

Tools Used

Manual review

Recommendations

Only do the division last after all the multiplications.

function getPendingFundingFeePerUnit(
Data storage self,
SD59x18 fundingRate,
UD60x18 markPriceX18
)
internal
view
returns (SD59x18)
{
- SD59x18 avgFundingRate = unary(sd59x18(self.lastFundingRate).add(fundingRate)).div(sd59x18Convert(2));
+ SD59x18 avgFundingRate = unary(sd59x18(self.lastFundingRate).add(fundingRate));
- return avgFundingRate.mul(getProportionalElapsedSinceLastFunding(self).intoSD59x18()).mul(
- markPriceX18.intoSD59x18()
- );
+ return avgFundingRate.mul(getProportionalElapsedSinceLastFunding(self).intoSD59x18()).mul(
+ markPriceX18.intoSD59x18()
+ ).div(sd59x18Convert(2));
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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