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

OI and skew updated incorrectly in liquidation

jennifersun

Summary

In liquidition process, the whole openInterest and skew will be changed and should be updated. The system lacks of this part.

Vulnerability Details

In liquidation process, the perpMarket.openInterest and perpMarket.skew will be changed after we finish liquidation. There two parameters should be updated.
Although we have called perpMarket.updateOpenInterest(ctx.newOpenInterestX18, ctx.newSkewX18);, actually ctx.newOpenInterestX18 and ctx.newSkewX18 is not initialized and keeps 0. This means that after one liquidation, the perpMarket.openInterest and perpMarket.skew will come to zero. This is incorrect and will increase the whole system's risk.

function liquidateAccounts(uint128[] calldata accountsIds) external {
......
// 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
// delete one active market for this account.
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);
}

Checking the previous cyfrin report 7.4.3, we delete checkOpenInterestLimits to avoid the possible dos. But the vulnerability is that once we delete this part, this will lead to ctx.newOpenInterestX18, ctx.newSkewX18 uninitialized.
One proper fix should be that we calculate the new OpenInterest and new skew and don't check ExceedsSkewLimit and ExceedsOpenInterestLimit.

- (ctx.newOpenInterestX18, ctx.newSkewX18) = perpMarket.checkOpenInterestLimits(
- ctx.liquidationSizeX18, ctx.oldPositionSizeX18, sd59x18(0), false
- );
​

Impact

We use maxSkew to control the difference between Long position and Short position. This is one system protection. Because of the incorrect OI and skew update, this protection will not take effect.

Tools Used

Manual

Recommendations

Calculate the new OpenInterest and new Skew and update correctly.

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!