Zaros Part 1
Zaros
DeFiFoundry
60,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Open interest and skew are set to 0 during liquidations

draiakoo

Summary

Open interest and skew are set to 0 during liquidations

Vulnerability Details

When a liquidation for a position is executed, all market open positions are intended to be removed and all the margin transfered to some protocol receivers. This is the coded implementation to liquidate each position:

​
struct LiquidationContext {
UD60x18 liquidationFeeUsdX18;
uint128 tradingAccountId;
SD59x18 marginBalanceUsdX18;
UD60x18 liquidatedCollateralUsdX18;
uint256[] activeMarketsIds;
uint128 marketId;
SD59x18 oldPositionSizeX18;
SD59x18 liquidationSizeX18;
UD60x18 markPriceX18;
SD59x18 fundingRateX18;
SD59x18 fundingFeePerUnitX18;
UD60x18 newOpenInterestX18;
SD59x18 newSkewX18;
}
​
​
function liquidateAccounts(uint128[] calldata accountsIds) external {
...
// initialize the context data struct in memory
LiquidationContext memory ctx;
​
​
// iterate over every account being liquidated; intentionally not caching
// length as reading from calldata is faster
for (uint256 i; i < accountsIds.length; i++) {
// store current accountId being liquidated in working data
ctx.tradingAccountId = accountsIds[i];
​
// sanity check for non-sensical accountId; should never be true
if (ctx.tradingAccountId == 0) continue;
​
// load account's leaf (data + functions)
TradingAccount.Data storage tradingAccount = TradingAccount.loadExisting(ctx.tradingAccountId);
​
// get account's required maintenance margin & unrealized PNL
(, UD60x18 requiredMaintenanceMarginUsdX18, SD59x18 accountTotalUnrealizedPnlUsdX18) =
tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(0, SD59x18_ZERO);
​
// get then save margin balance into working data
ctx.marginBalanceUsdX18 = tradingAccount.getMarginBalanceUsd(accountTotalUnrealizedPnlUsdX18);
​
// if account is not liquidatable, skip to next account
// account is liquidatable if requiredMaintenanceMarginUsdX18 > ctx.marginBalanceUsdX18
if (!TradingAccount.isLiquidatable(requiredMaintenanceMarginUsdX18, ctx.marginBalanceUsdX18)) {
continue;
}
​
// deduct maintenance margin from the account's collateral
// settlementFee = liquidationFee
ctx.liquidatedCollateralUsdX18 = tradingAccount.deductAccountMargin({
feeRecipients: FeeRecipients.Data({
marginCollateralRecipient: globalConfiguration.marginCollateralRecipient,
orderFeeRecipient: address(0),
settlementFeeRecipient: globalConfiguration.liquidationFeeRecipient
}),
​
// this will transfer all the liquidated tokens to a recipient
pnlUsdX18: requiredMaintenanceMarginUsdX18,
orderFeeUsdX18: UD60x18_ZERO,
settlementFeeUsdX18: ctx.liquidationFeeUsdX18
});
​
// clear pending order for account being liquidated
MarketOrder.load(ctx.tradingAccountId).clear();
​
// copy active market ids for account being liquidated
ctx.activeMarketsIds = tradingAccount.activeMarketsIds.values();
​
// 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);
}
​
emit LogLiquidateAccount(
msg.sender,
ctx.tradingAccountId,
ctx.activeMarketsIds.length,
requiredMaintenanceMarginUsdX18.intoUint256(),
ctx.marginBalanceUsdX18.intoInt256(),
ctx.liquidatedCollateralUsdX18.intoUint256(),
ctx.liquidationFeeUsdX18.intoUint128()
);
}
}

Looking closely to the sequence of actions that this function does we can sum up like this:

  1. Check if the position's margin plus unrealized PNL can back all opened positions

  2. If the position is liquidable transfer the liquidation fee to the receiver and transfer the remaining margin funds to marginCollateralRecipient

  3. Clear position market order

  4. For each active market that the user had do the following actions:
    4.1. Close the position
    4.2. Update funding rates
    4.3. Update active markets for the positions
    4.4. Update open interest and skew for the market

This process works fine apart from the update of open interest and skew. If you look closely to the implementation it just calls the function updateOpenInterest with the variables ctx.newOpenInterestX18 and ctx.newSkewX18. If you try to find where these variables are set within the liquidate function you will notice that they are not set during the whole execution, that means that they will be 0. And when it calls updateOpenInterest it will reset and override the open interest and skew for each market of the position being liquidate.

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

This issue has a high impact for the protocol because the skew is used to balance the amount of short and long positions in the protocol and to incentive its balancing via maker/taker fees and constraining a maximum allowed skew. The open interest is also a security measure to set a maximum amount of open size, so if it gets reset, this protection will be completely useless and will not take effect.
It is true that this issue only happens for the active markets of a position being liquidated, but a user can open a size in all markets in the protocol on purpose and when he will get automatically liquidated, it will reset all markets open interest and skew.

Impact

High impact, these 2 variables are really important for the protocol integrity and are reset automatically when positions get liquidated, so it will happen often.

Tools Used

Manual review

Recommendations

Compute the open interest and the skew before updating them:

function liquidateAccounts(uint128[] calldata accountsIds) external {
...
// 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;
​
+ UD60x18 currentOpenInterest = ud60x18(perpMarket.openInterest);
+ ctx.newOpenInterestX18 = currentOpenInterest.sub(ctx.liquidationSizeX18.abs().intoUD60x18()).intoUD60x18();
+ SD59x18 currentSkew = sd59x18(perpMarket.skew);
+ ctx.newSkewX18 = currentSkew.add(ctx.liquidationSizeX18);
​
​
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.