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

Liquidation will unexpectedly set market's open interest and skew to zero

shaflow01

Summary

During the liquidation process, the variables newOpenInterestX18 and newSkewX18 were not calculated, but the updateOpenInterest function was called to update market's open interest and sketch, which resulted in them being unexpectedly set to zero.

Vulnerability Details

During the liquidation process, the active markets of the liquidated account will be traversed, and the positions of the liquidated account will be cloesd.

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

​
function checkOpenInterestLimits(
Data storage self,
SD59x18 sizeDelta,
SD59x18 oldPositionSize,
SD59x18 newPositionSize
)
internal
view
returns (UD60x18 newOpenInterest, SD59x18 newSkew)
{
...
​
}

Liquidation does not call the checkOpenInterestLimits function to verify the market's open interest and skew limits.

This is because we do not want the liquidation to be terminated by these limits.

However, it should be noted that not calling checkOpenInterestLimits also means that newOpenInterest and newSkew are not calculated.

returns (UD60x18 newOpenInterest, SD59x18 newSkew)

Ultimately, due to not calculating newOpenInterest and newSkew, the liquidation function calling updateOpenInterest function will unexpectedly set the market's open interest and skew to zero.

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/LiquidationBranch.sol#L209C28-L209C46

/// @param accountsIds The list of accounts to liquidate
function liquidateAccounts(uint128[] calldata accountsIds) external {
...
for (uint256 i; i < accountsIds.length; i++) {
...
for (uint256 j; j < ctx.activeMarketsIds.length; j++) {
...
​
// 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
/// @notice 更新账户的活跃市场；这将调用 EnumerableSet::remove，因此我们迭代的是交易者活跃市场的内存副本
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 market's open interest and skew will be incorrectly set to zero, causing checkOpenInterestLimits to fail and lose its restrictions.

Tools Used

Manual review

Recommendations

Set up a function in PerpMarket contract to calculate the new open interest and skew of the per market withOutChechLimit for liquidation

function getNewOpenInterestWithOutChechLimit(
Data storage self,
SD59x18 sizeDelta,
SD59x18 oldPositionSize,
SD59x18 newPositionSize
)
internal
view
returns (UD60x18 newOpenInterest, SD59x18 newSkew)
{
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());
​
// int128 -> SD59x18
SD59x18 currentSkew = sd59x18(self.skew);
​
// calculate new skew
newSkew = currentSkew.add(sizeDelta);
​
}
/// @param accountsIds The list of accounts to liquidate
function liquidateAccounts(uint128[] calldata accountsIds) external {
...
for (uint256 i; i < accountsIds.length; i++) {
...
for (uint256 j; j < ctx.activeMarketsIds.length; j++) {
...
​
// 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
/// @notice 更新账户的活跃市场；这将调用 EnumerableSet::remove，因此我们迭代的是交易者活跃市场的内存副本
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
+ (ctx.newOpenInterestX18, ctx.newSkewX18) =
+ perpMarket.getNewOpenInterestWithOutChechLimit(ctx.oldPositionSizeX18, ctx.oldPositionSizeX18, SD59x18_ZERO);
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.