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

Incorrect updates to the market's open interest and skew in function liquidateAccounts

Summary

Incorrect updates to the market's open interest and skew in function liquidateAccounts. To fix this, the function should calculate the new open interest and skew based on the liquidation size.

Vulnerability Details

// iterate over memory copy of active market ids
// intentionally not caching length as expected size < 3 in most cases
for (uint256 j; j < ctx.activeMarketsIds.length; j++) {
// load current active market id into working data
ctx.marketId = ctx.activeMarketsIds[j].toUint128();
// fetch storage slot for perp market
PerpMarket.Data storage perpMarket = PerpMarket.load(ctx.marketId);
// load position data for user being liquidated in this market
Position.Data storage position = Position.load(ctx.tradingAccountId, ctx.marketId);
// save open position size
ctx.oldPositionSizeX18 = sd59x18(position.size);
// save inverted sign of open position size to prepare for closing the position
ctx.liquidationSizeX18 = -ctx.oldPositionSizeX18;
// calculate price impact of open position being closed
ctx.markPriceX18 = perpMarket.getMarkPrice(ctx.liquidationSizeX18, perpMarket.getIndexPrice());
// get current funding rates
ctx.fundingRateX18 = perpMarket.getCurrentFundingRate();
ctx.fundingFeePerUnitX18 = perpMarket.getNextFundingFeePerUnit(ctx.fundingRateX18, ctx.markPriceX18);
// update funding rates for this perpetual market
perpMarket.updateFunding(ctx.fundingRateX18, ctx.fundingFeePerUnitX18);
// reset the position
position.clear();
// update account's active markets; this calls EnumerableSet::remove which
// is why we are iterating over a memory copy of the trader's active markets
tradingAccount.updateActiveMarkets(ctx.marketId, ctx.oldPositionSizeX18, SD59x18_ZERO);
// update perp market's open interest and skew; we don't enforce ipen
// interest and skew caps during liquidations as:
// 1) open interest and skew are both decreased by liquidations
// 2) we don't want liquidation to be DoS'd in case somehow those cap
// checks would fail
perpMarket.updateOpenInterest(ctx.newOpenInterestX18, ctx.newSkewX18);
}

Impact

The function is using ctx.newOpenInterestX18 and ctx.newSkewX18 to update the open interest and skew of the perpetual market. However, these variables are never defined or calculated in the function.

This error will lead to:

  1. Incorrect updates to the market's open interest and skew.

  2. Potential use of uninitialized variables, which in Solidity default to zero values.

Tools Used

Manual Review

Recommendations

// Calculate new open interest and skew

ctx.newOpenInterestX18 = perpMarket.openInterestX18.sub(ctx.oldPositionSizeX18.abs());

ctx.newSkewX18 = perpMarket.skewX18.sub(ctx.oldPositionSizeX18);

// Update perp market's open interest and skew

perpMarket.updateOpenInterest(ctx.newOpenInterestX18, ctx.newSkewX18);

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

`liquidateAccounts` calls `updateOpenInterest` with uninitialized OI and skew)

Support

FAQs

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