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

Wrong calculation of accrued funding when `skew>0` in `Position::getAccuredFunding`

Summary

When a user's order gets filled their profits/losses from the funding fee get calculated in order to verify the position. The problem arises in the fact that when the skew is positive and was also positive for the last trade the user did their accruedFunding will be doubled than what it should be meaning they will loose or receive double the fees.

Vulnerability Details

First the funding fee per unit is calculated and updated for the market:

// get funding rates for this perp market
ctx.fundingRateX18 = perpMarket.getCurrentFundingRate();
ctx.fundingFeePerUnitX18 = perpMarket.getNextFundingFeePerUnit(ctx.fundingRateX18, fillPriceX18);
// update funding rates
perpMarket.updateFunding(ctx.fundingRateX18, ctx.fundingFeePerUnitX18);

The sign of the funding rate per unit will generally be opposite of the value of the skew if the skew hasn't switched signs (e.g positive skew => negative fundingFeePerUnit).

Then the unrealized pnl is calculated by calling TradingAccount::getAccountMarginRequirementUsdAndUnrealizedPnlUsd. Where the accrued funding (profit / losses from funding fee) is calculated.

// calculate required initial & maintenance margin for this trade
// and account's unrealized PNL
(
UD60x18 requiredInitialMarginUsdX18,
UD60x18 requiredMaintenanceMarginUsdX18,
SD59x18 accountTotalUnrealizedPnlUsdX18
) = tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(marketId, sizeDeltaX18);
// get unrealized pnl + accrued funding fees
SD59x18 positionUnrealizedPnl =
@> position.getUnrealizedPnl(markPrice).add(position.getAccruedFunding(fundingFeePerUnit));

Finally if we take a look at the function to calculated the accrued funding we see that we subtract the old funding fee from the new one. The logic is generally correct if both values are positive but in the situations where self.lastInteractionFundingFeePerUnit < 0 that would actually lead to increasing the funding fee.

Under normal situation: self.lastInteractionFundingFeePerUnit = -0. 1; fundingFeePerUnit = -0.05; = > netFundingFeePerUnit = -0.05 ...
But in the current code: self.lastInteractionFundingFeePerUnit = -0. 1; fundingFeePerUnit = -0.05; = > netFundingFeePerUnit = -0.15 ...

/// @notice Returns the accrued funding fee.
/// @param self The position storage pointer.
/// @param fundingFeePerUnit The market's current funding fee per unit.
/// @return accruedFundingUsdX18 The accrued funding fee, positive or negative.
function getAccruedFunding(
Data storage self,
SD59x18 fundingFeePerUnit
)
internal
view
returns (SD59x18 accruedFundingUsdX18)
{
@> SD59x18 netFundingFeePerUnit = fundingFeePerUnit.sub(sd59x18(self.lastInteractionFundingFeePerUnit));
accruedFundingUsdX18 = sd59x18(self.size).mul(netFundingFeePerUnit);
}

Impact

Calculation of the accrued funding is wrong when the skew is > 0 which leads to loss of funds for the contract and users.

Tools Used

Manual Review

Recommendations

Check if fundingFeePerUnit and self.lastInteractionFundingFeePerUnit are both less than 0 and adjust the formula for that case by changing sub to add

Updates

Lead Judging Commences

inallhonesty Lead Judge
11 months ago
inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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