Summary
PerpMarket::getPendingFundingFeePerUnit is vunerable to precision loss due to division before multiplication.
Vulnarability Details
When calculating the next funding fee per unit, the PerpMarket::getNextFundingFeePerUnit function calls the PerpMarket::getPendingFundingFeePerUnit:
File: src/perpetuals/leaves/PerpMarket.sol
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()
);
}
A multiplication is performed on avgFundingRate is used in the return statement. But the its value is obtained in the statement prior to the return statement by performing a division.
This calculation does division before multiplication which results in a small loss of precision (the smaller are self.lastFundingRate and fundingRate, the greater is the loss of precision) and leads to an incorrect result.
Proof Of Concept
Replace the PerpMarket::getPendingFundingFeePerUnit function with the one below:
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 unary(sd59x18(self.lastFundingRate).add(fundingRate)).mul(
getProportionalElapsedSinceLastFunding(self).intoSD59x18()).mul(
markPriceX18.intoSD59x18()
).div(sd59x18Convert(2));
}
Replace the GetFundingRate_Integration_Test::testFuzz_GetPendingFundingFeePerUnit function with the one below:
import { PerpMarket } from "@zaros/perpetuals/leaves/PerpMarket.sol";
import { PerpMarketHarness } from "test/harnesses/perpetuals/leaves/PerpMarketHarness.sol";
import { UD60x18, convert as ud60x18Convert, ud60x18 } from "@prb-math/UD60x18.sol";
import { SD59x18, sd59x18, unary, convert as sd59x18Convert } from "@prb-math/SD59x18.sol";
import { console } from "forge-std/console.sol";
function testFuzz_GetPendingFundingFeePerUnit(
uint128 marketId,
uint256 marginValueUsd,
bool isLong,
uint256 timeElapsed
)
external
{
MarketConfig memory fuzzMarketConfig = getFuzzMarketConfig(marketId);
marginValueUsd = bound({
x: marginValueUsd,
min: USDC_MIN_DEPOSIT_MARGIN,
max: convertUd60x18ToTokenAmount(address(usdc), USDC_DEPOSIT_CAP_X18)
});
timeElapsed = bound({ x: timeElapsed, min: 1 seconds, max: 365 days });
deal({ token: address(usdc), to: users.naruto.account, give: marginValueUsd });
uint256 initialMarginRate = ud60x18(fuzzMarketConfig.imr).mul(ud60x18(1.001e18)).intoUint256();
uint128 tradingAccountId = createAccountAndDeposit(marginValueUsd, address(usdc));
openPosition(fuzzMarketConfig, tradingAccountId, initialMarginRate, marginValueUsd, isLong);
skip(timeElapsed);
PerpMarketHarness perpMarketHarness = PerpMarketHarness(address(perpsEngine));
UD60x18 indexPriceX18 = ud60x18(fuzzMarketConfig.mockUsdPrice);
UD60x18 markPrice = perpsEngine.getMarkPrice(fuzzMarketConfig.marketId, indexPriceX18.intoUint256(), 0);
SD59x18 fundingRate = perpsEngine.getFundingRate(fuzzMarketConfig.marketId);
PerpMarket.Data memory perpMarket = perpMarketHarness.exposed_PerpMarket_load(fuzzMarketConfig.marketId);
SD59x18 avgFundingRate = unary(sd59x18(perpMarket.lastFundingRate).add(fundingRate)).div(sd59x18Convert(2));
int256 expectedPendingFundingFeePerUnit = avgFundingRate.mul(ud60x18Convert(timeElapsed).intoSD59x18()).mul(
markPrice.intoSD59x18()
).intoInt256();
SD59x18 pendingFundingFeePerUnit = perpMarketHarness.exposed_getPendingFundingFeePerUnit(
fuzzMarketConfig.marketId,
fundingRate,
markPrice
);
console.log("pendingFundingFeePerUnit", pendingFundingFeePerUnit.intoInt256());
console.log("expectedPendingFundingFeePerUnit", expectedPendingFundingFeePerUnit);
assertAlmostEq(pendingFundingFeePerUnit.intoInt256(), expectedPendingFundingFeePerUnit, 1);
}
Run forge test --mt testFuzz_GetPendingFundingFeePerUnit -vv.
Impact
The resulting pending funding fees per unit are incorrect. They are used to obtained the next funding fees per unit which are used to get the an account's margin requirements and unrealized PNL and also used to get a position state. All those values will be incorrect and so will be the subsequent result of the calculation using them.
Tools Used
Manual review.
Recommendations
Simplify the calculation to:
return unary(sd59x18(self.lastFundingRate).add(fundingRate)).mul(
getProportionalElapsedSinceLastFunding(self).intoSD59x18()).mul(
markPriceX18.intoSD59x18()
).div(sd59x18Convert(2));
