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

Liquidation process resets skew back to zero, resulting in an extreme imbalance in the perpetual contract price.

Summary

Vulnerability Details

In perpetual futures, skew refers to the imbalance between long and short positions. This factor is used to determine the mark price of the contract in a given market, at which the trade is filled. The algorithm incentivizes trades (in maker/taker fees) if it shifts the skew towards zero.

The vulnerability lies in LiquidationBranch, where on liquidating accounts for a given market results in the skew being reset to zero. In the following snippet, ctx.newOpenInterestX18 and ctx.newSkewX18 has never been modified, as a result default value 0 is get updated to their storage slot.
https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/LiquidationBranch.sol#L209

function liquidateAccounts(uint128[] calldata accountsIds) external {
...snip...
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);
...snip...
// update perp market's open interest and skew;
perpMarket.updateOpenInterest(ctx.newOpenInterestX18, ctx.newSkewX18); // @audit ctx.newSkewX18 == 0 and ctx.newOpenInterestX18 == 0
}
}
}

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

function updateOpenInterest(Data storage self, UD60x18 newOpenInterest, SD59x18 newSkew) internal {
self.skew = newSkew.intoInt256().toInt128();
self.openInterest = newOpenInterest.intoUint128();
}

Impact

Wrong skew value for a given market has many severe impacts throughout the protocol, one of which is incorrect pricing while filling orders,
https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/SettlementBranch.sol#L153

function fillMarketOrder(
uint128 tradingAccountId,
uint128 marketId,
bytes calldata priceData
)
external
onlyMarketOrderKeeper(marketId)
{
...snip...
// verify the provided price data against the verifier and ensure it's valid, then get the mark price
// based on the returned index price.
ctx.fillPriceX18 = perpMarket.getMarkPrice(ctx.sizeDeltaX18, ctx.indexPriceX18); // @audit incorrect mark price
// perform the fill
_fillOrder(
tradingAccountId,
marketId,
SettlementConfiguration.MARKET_ORDER_CONFIGURATION_ID,
ctx.sizeDeltaX18,
ctx.fillPriceX18
);
// reset pending order details
}

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

function getMarkPrice(
Data storage self,
SD59x18 skewDelta,
UD60x18 indexPriceX18
)
internal
view
returns (UD60x18 markPrice)
{
SD59x18 skewScale = sd59x18(self.configuration.skewScale.toInt256());
SD59x18 skew = sd59x18(self.skew);
SD59x18 priceImpactBeforeDelta = skew.div(skewScale);
SD59x18 newSkew = skew.add(skewDelta);
SD59x18 priceImpactAfterDelta = newSkew.div(skewScale);
SD59x18 cachedIndexPriceX18 = indexPriceX18.intoSD59x18();
UD60x18 priceBeforeDelta =
cachedIndexPriceX18.add(cachedIndexPriceX18.mul(priceImpactBeforeDelta)).intoUD60x18();
UD60x18 priceAfterDelta =
cachedIndexPriceX18.add(cachedIndexPriceX18.mul(priceImpactAfterDelta)).intoUD60x18();
markPrice = priceBeforeDelta.add(priceAfterDelta).div(ud60x18Convert(2));
}

Tools Used

Manual

Recommendations

Modify ctx.newOpenInterestX18 and ctx.newSkewX18 according to the change in the position size of the account.

Updates

Lead Judging Commences

inallhonesty Lead Judge over 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.

Give us feedback!