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

Incorrect Open Interest and Skew Update in `LiquidationBranch::liquidateAccounts(...)` Leading to Market Disruption

aamirusmani1552

Summary

The LiquidationBranch::liquidateAccounts(...) function clears out the Market's open interest and skew, causing multiple issues.

Vulnerability Details

The LiquidationBranch::liquidateAccounts(...) function attempts to update the current market's open interest and skew at the end of the function using ctx.newOpenInterestX18 and ctx.newSkewX18 from the current context. However, these values are never updated within the function, meaning they retain their default value of 0. Consequently, the function updates the open interest with these zero values, making both the open interest and skew for the perp market empty.

function liquidateAccounts(uint128[] calldata accountsIds) external {
​
...
​
@> 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()
);
}
}

GitHub: [209]

This leads to two main issues:

  1. If someone with an active position in the market tries to create a new order, it will always revert when the open interest limits are checked due to underflow:

function checkOpenInterestLimits(
Data storage self,
SD59x18 sizeDelta,
SD59x18 oldPositionSize,
SD59x18 newPositionSize
)
internal
view
returns (UD60x18 newOpenInterest, SD59x18 newSkew)
{
// load max & current open interest for this perp market uint128 -> UD60x18
UD60x18 maxOpenInterest = ud60x18(self.configuration.maxOpenInterest);
UD60x18 currentOpenInterest = ud60x18(self.openInterest);
​
// calculate new open interest which would result from proposed trade
// by subtracting old position size then adding new position size to
// current open interest
@> newOpenInterest =
currentOpenInterest.sub(oldPositionSize.abs().intoUD60x18()).add(newPositionSize.abs().intoUD60x18());
​
...
​
}
​
GitHub: [[293-297](https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/leaves/PerpMarket.sol#L287C7-L293C20)]

The underflow occurs because the currentOpenInterest value is 0, and it is of type UD60x18, which does not handle signed values. When subtracting oldPositionSize (which is not0 since there is an open position) from currentOpenInterest, it underflows, causing the function to revert.

  1. When a new user tries to open a position in the market, the funding rate, mark price, etc., might not favor them as the current skew is 0

The mark price is calculated like this:

let proportionalSkew = prevSkew / skewScale
let newSkew = prevSkew + changeInOpenInterest // Can be both + and negative
​
let newProportionalSkew = newSkew / skewScale
let priceImpactBeforeTrade = indexPrice + (indexPrice * proportionalSkew)
let priceImpaceAfterTrade = indexPrice + (indexPrice * newProportionalSkew)
// Now we will calculate the markPrice
let markPrice = (priceImpactBeforeTrade + priceImpactAfterTrade) / 2

Example calculation with incorrect skew:

  • Assume the open interest before liquidation was -10000 and the index price is 1 USD. After liquidation, which liquidated the short position of 5000, the new skew should be -5000, but it will be 0. If a new user tries to create a long position of 2000, they should get a discount, but this is not the case.

let proportionalSkew = prevSkew / skewScale // => 0 / 10000000 => 0 considering skew scale is 10 million
let newSkew = prevSkew + changeInOpenInterest // => 0 + 2000 => 2000
​
let newProportionalSkew = newSkew / skewScale // => 2000 / 10000000 => 0.0002
let priceImpactBeforeTrade = indexPrice + (indexPrice * proportionalSkew) // => 1 + (1 * 0) => 1
let priceImpaceAfterTrade = indexPrice + (indexPrice * newProportionalSkew) // => 1 + (1 * 0.0002) => 1.0002
// Now we will calculate the markPrice
let markPrice = (priceImpactBeforeTrade + priceImpactAfterTrade) / 2 // => 1 + 1.0002 / 2 => 1.0001

So mark price is 1.0001usd.

  • Let's take a look at the same calculation with correct skew:

let proportionalSkew = prevSkew / skewScale // => -5000 / 10000000 => -0.0005 considering skew scale is 10 million
let newSkew = prevSkew + changeInOpenInterest // => -5000 + 2000 => -3000
​
let newProportionalSkew = newSkew / skewScale // => -3000 / 10000000 => -0.0003
let priceImpactBeforeTrade = indexPrice + (indexPrice * proportionalSkew) // => 1 + (1 * -0.0005) => 0.9995
let priceImpaceAfterTrade = indexPrice + (indexPrice * newProportionalSkew) // => 1 + (1 * -0.0003) => 0.9997
// Now we will calculate the markPrice
let markPrice = (priceImpactBeforeTrade + priceImpactAfterTrade) / 2 // => 0.9995 + 0.9997 / 2 => 0.9996

The trade should execute at 0.9996 USD, a discounted rate, but currently, it is executed at 1.0001 USD, causing the user to pay a premium. Incorrect skew also affects funding velocity, funding rate, etc., leading to potential user losses or profits.

Impact

Old users cannot create new positions, and new users might incur losses or profits.

Proof Of Concept

function test_LiquidationWillClearOpenInterestAndSkew() public{
// setup
uint128 marketId = 1; // btcusd market id
address token = address(wBtc);
int128 tokenDecimals = 8;
uint256 amount = 1 * 10 ** uint128(tokenDecimals);
int128 sizeDelta = int128(int128(50) * int128(10) ** uint128(18));
​
// minting tokens to naruto and sasuke
deal({ token: token, to: users.naruto.account, give: amount });
deal({ token: token, to: users.sasuke.account, give: amount });
​
//////////////////////////////////////////////////
/// Naruto opens a position ///
//////////////////////////////////////////////////
​
// create account
uint128 narutoTrandingAccountId = createAccountAndDeposit(amount, token);
​
// open position
uint256[2] memory marketsIdsRange;
marketsIdsRange[0] = marketId;
marketsIdsRange[1] = marketId;
​
MarketConfig[] memory filteredMarketsConfig = getFilteredMarketsConfig(marketsIdsRange);
​
​
perpsEngine.createMarketOrder(
OrderBranch.CreateMarketOrderParams({
tradingAccountId: narutoTrandingAccountId,
marketId: filteredMarketsConfig[0].marketId,
sizeDelta: sizeDelta
})
);
​
skip(3 minutes);
​
// fill the order
bytes memory mockSignedReport =
getMockedSignedReport(marketsConfig[marketId].streamId, marketsConfig[marketId].mockUsdPrice);
​
vm.startPrank({ msgSender: marketOrderKeepers[marketId] });
​
// fill first order and open position
perpsEngine.fillMarketOrder(narutoTrandingAccountId, marketId, mockSignedReport);
​
skip(1 hours);
​
/////////////////////////////////////////////////////////////
/// Sasuke opens a position ///
////////////////////////////////////////////////////////////
​
vm.startPrank({msgSender: users.sasuke.account});
// sasuke also opens a position
uint128 sasukeTradingAccountId = createAccountAndDeposit(amount, token);
​
perpsEngine.createMarketOrder(
OrderBranch.CreateMarketOrderParams({
tradingAccountId: sasukeTradingAccountId,
marketId: filteredMarketsConfig[0].marketId,
sizeDelta: sizeDelta
})
);
​
skip(3 minutes);
​
// fill the order
​
vm.startPrank({ msgSender: marketOrderKeepers[marketId] });
​
// fill first order and open position
mockSignedReport =
getMockedSignedReport(marketsConfig[marketId].streamId, marketsConfig[marketId].mockUsdPrice);
​
perpsEngine.fillMarketOrder(sasukeTradingAccountId, marketId, mockSignedReport);
​
// getting open interest in the market
(UD60x18 longsOpenInterest, UD60x18 shortsOpenInterest, UD60x18 totalOpenInterest) = perpsEngine.getOpenInterest(marketId);
​
console2.log("longsOpenInterest", longsOpenInterest.intoUint256());
console2.log("shortsOpenInterest", shortsOpenInterest.intoUint256());
console2.log("totalOpenInterest", totalOpenInterest.intoUint256());
​
// make the naruto's account liquidatable
updateMockPriceFeed(marketId, 60000e8);
​
// liquidate the account
uint128[] memory accountsIds = new uint128[](1);
accountsIds[0] = narutoTrandingAccountId;
​
vm.startPrank({ msgSender: liquidationKeeper });
perpsEngine.liquidateAccounts(accountsIds);
​
// again getting the open interest in the market
(longsOpenInterest, shortsOpenInterest, totalOpenInterest) = perpsEngine.getOpenInterest(marketId);
​
console2.log("longsOpenInterest", longsOpenInterest.intoUint256());
console2.log("shortsOpenInterest", shortsOpenInterest.intoUint256());
console2.log("totalOpenInterest", totalOpenInterest.intoUint256());
​
//////////////////////////////////////////////////////////////////////
// Sasuke places another order but will revert //
/////////////////////////////////////////////////////////////////////
skip(1 hours);
​
deal({ token: token, to: users.sasuke.account, give: amount });
​
vm.startPrank({msgSender: users.sasuke.account});
// create another order
​
vm.expectRevert();
perpsEngine.createMarketOrder(
OrderBranch.CreateMarketOrderParams({
tradingAccountId: sasukeTradingAccountId,
marketId: filteredMarketsConfig[0].marketId,
sizeDelta: sizeDelta
})
);
}

Tools Used

  • Manual Review

  • Foundry

Recommendations

It is recommended to update the context values to new open interest and skew and then make call to the PerpMarket.updateOpenInterest(...)

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.