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

Lack of new Open Interest recalculation during liquidations results in a protocol breakdown

0x5chn0uf

Summary

The LiquidationBranch::liquidateAccounts function does not calculate the new open interest and new skew when liquidating positions. Thus, it sets the open interest to zero, regardless of the actual state of open positions in the market. This occurs because the ctx.newOpenInterestX18 and ctx.newSkewX18 variables are not being calculated or updated before calling perpMarket.updateOpenInterest().

Vulnerability Details

function liquidateAccounts(uint128[] calldata accountsIds) external {
// ...
​
// working data
LiquidationContext memory ctx;
​
// ..
for (uint256 i; i < accountsIds.length; i++) {
// ...
for (uint256 j; j < ctx.activeMarketsIds.length; j++) {
// ...
// @audit: Value are never computed beforehand (defaulting to 0)
@> perpMarket.updateOpenInterest(ctx.newOpenInterestX18, ctx.newSkewX18);
}

Proof of Concept

  1. Open positions:

    • Naruto opens a long position of 1 BTC.

    • Sasuke opens a short position of 1 BTC.

  2. Price fluctuates in one side and trigger one liquidation

  3. After performing the liquidation, the open interest is incorrectly set to 0, while the other position still exists.

  4. Attempting to close the remaining position fails, causing the protocol to panic due to insufficient open interest, even though the position still exists.

Proof of Code

Add the following test in test/integration/perpetuals/liquidation-branch/liquidateAccounts/liquidateAccounts.t.sol

function test_IncorrectOpenInterestAfterLiquidation() public {
MarketConfig memory fuzzMarketConfig = getFuzzMarketConfig(BTC_USD_MARKET_ID);
MockPriceFeed priceFeed = MockPriceFeed(fuzzMarketConfig.priceAdapter);
​
// Setup two users with USDC
deal({ token: address(usdc), to: users.naruto.account, give: 1000e6 });
deal({ token: address(usdc), to: users.sasuke.account, give: 1000e6 });
​
uint256 marginValueUsd = 100e6;
changePrank({ msgSender: users.naruto.account });
uint128 narutoAccountId = createAccountAndDeposit(marginValueUsd, address(usdc));
changePrank({ msgSender: users.sasuke.account });
uint128 sasukeAccountId = createAccountAndDeposit(marginValueUsd, address(usdc));
​
// Open positions
changePrank({ msgSender: users.naruto.account });
openPosition(fuzzMarketConfig, narutoAccountId, fuzzMarketConfig.imr, marginValueUsd, true);
changePrank({ msgSender: users.sasuke.account });
openPosition(fuzzMarketConfig, sasukeAccountId, fuzzMarketConfig.imr, marginValueUsd, false);
​
(UD60x18 longsOpenInterest, UD60x18 shortsOpenInterest, UD60x18 totalOpenInterest) = perpsEngine.getOpenInterest(
fuzzMarketConfig.marketId
);
​
// Assert that open interest is correct after opening positions
assertEq(
longsOpenInterest.intoUint256(),
shortsOpenInterest.intoUint256(),
"Long and short open interest should be equal"
);
assertEq(
totalOpenInterest.intoUint256(),
longsOpenInterest.intoUint256() + shortsOpenInterest.intoUint256(),
"Total open interest should be sum of long and short open interest"
);
​
// Decrease price to trigger liquidation
uint256 newPrice = fuzzMarketConfig.mockUsdPrice / 100; // 99% price decrease
priceFeed.updateMockPrice(newPrice);
​
// Check for liquidations
uint128[] memory liquidatableAccountsIds = perpsEngine.checkLiquidatableAccounts(0, 2);
assertEq(liquidatableAccountsIds.length, 1, "Expected one liquidatable account");
assertEq(liquidatableAccountsIds[0], narutoAccountId, "Expected Naruto's account to be liquidatable");
​
// Perform liquidation
changePrank({ msgSender: liquidationKeeper });
perpsEngine.liquidateAccounts(liquidatableAccountsIds);
​
(longsOpenInterest, shortsOpenInterest, totalOpenInterest) = perpsEngine.getOpenInterest(fuzzMarketConfig.marketId);
// Assert that open interest is zero after liquidation
assertEq(longsOpenInterest.intoUint256(), 0, "Long open interest is zero after liquidation");
assertEq(shortsOpenInterest.intoUint256(), 0, "Short open interest is also zero after liquidation");
assertEq(totalOpenInterest.intoUint256(), 0, "Total open interest is set to zero after liquidation");
​
// Attempt to close Sasuke's short position
changePrank({ msgSender: users.sasuke.account });
// We revert with arithmetic underflow because OI is a SD60X18 type
// Indeed, by closing the position, we're going to do 0 - fuzzMarketConfig.minTradeSize
vm.expectRevert(stdError.arithmeticError);
perpsEngine.createMarketOrder(
OrderBranch.CreateMarketOrderParams({
tradingAccountId: sasukeAccountId,
marketId: fuzzMarketConfig.marketId,
sizeDelta: int128(fuzzMarketConfig.minTradeSize)
})
);
}

Impact

This issue has severe implications for the protocol:

  1. Market Integrity: The open interest being incorrectly set to zero misrepresents the true state of the market, leading to incorrect risk assessments.

  2. Protocol Instability: Users wouldn't be able to close their positions, as the system would incorrectly believe that no open interest exists. This could lead to:

    1. Trapped funds: Users unable to exit their positions, potentially leading to significant losses.

    2. Loss of trust: Users may lose confidence in the protocol, potentially causing a mass exodus and liquidity crisis.

Tools Used

Foundry Invariant Testing + Stress Test of the protocol

Recommendations

To address this issue, implement the following changes in the LiquidationBranch::liquidateAccounts function:

+ (ctx.newOpenInterestX18, ctx.newSkewX18) = perpMarket.checkOpenInterestLimits(ctx.liquidationSizeX18, ctx.oldPositionSizeX18, SD59x18_ZERO);
​
// Now update the 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.