Submission Details
Severity:
Resetting OI and skew down to 0 after liquidating 1 position, no matter if there are other open positions with longs and shorts on the same Perpetual Market
## Summary
When updating the OpenInterests and Skew during liquidation, the logic incorrectly passes the default value (0) for the two variables, which causes the skew and openInterest to be set to 0, regardless of whether there are shorts and longs opened on the Perpetual Market.
## Vulnerability Details
As part of the liquidation process, the OpenInterests and Skew need to be updated to reflect the change in the market caused by the liquidation. [The problem is that the values for the newOpenInterests and the newSkew are actually never computed](https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/branches/LiquidationBranch.sol#L200-L202), they are only declared and left with the default value. This causes that when the [`perpMarket.updateOpenInterest()`](https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/leaves/PerpMarket.sol#L354-L357) is called, the new values will be set to 0, instead of setting the new values to the correct amount that should reflect the change on the market after closing the liquidated position.
```
> LiquidationBranch.sol
/// @param accountsIds The list of accounts to liquidate
function liquidateAccounts(uint128[] calldata accountsIds) external {
...
// working data
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++) {
...
for (uint256 j; j < ctx.activeMarketsIds.length; j++) {
...
//@audit-issue => `ctx.newOpenInterestX18` && `ctx.newSkewX18` are never computed
//@audit-issue => They will be sent as 0 to the updateOpenInterest(), which will cause the skew and openInterests to be set to 0!
// 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);
}
...
}
}
> PerpMarket.sol
//@audit-issue => Will set both values to 0, regardless of whether there are longs and shorts opened on the Perpetual Market!
function updateOpenInterest(Data storage self, UD60x18 newOpenInterest, SD59x18 newSkew) internal {
self.skew = newSkew.intoInt256().toInt128();
self.openInterest = newOpenInterest.intoUint128();
}
```
## Impact
Updating the skew down to 0 causes multiple values to be computed totally wrong, for example, the fundingVelocity, and the MarkPrice, which, ultimately ends up causing the whole accounting to be messed up and not representing accurate values to reflect the real status of the Perpetual Market.
## Tools Used
Manual Audit
## Recommendations
Similar to how the [`SettlementBranch._fillOrder() function`]() updates the OI and the skew, by first calling the [`PerpetualMarket.checkOpenInterestLimits() function`](https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/branches/SettlementBranch.sol#L454-L460) to compute the actual new values based on the sizeDelta of the position, create a new helper function that will also do the same computations with the negative full size of the position (representing the position is closed), and, make sure to call this new helper function to get the correct new values for the OI and skew.
- Since liquidations should not be DoSed by an edge case that could be caused because of the validations on the `PerpetualMarket.checkOpenInterestLimits() function`, best opt to create a new function that only does the computation without the checks to enforce OI and skew limits.
Updates
Lead Judging Commences
inallhonesty over 1 year ago
Submission Judgement Published Assigned finding tags:
`liquidateAccounts` calls `updateOpenInterest` with uninitialized OI and skew)
Prize pool breakdown
Total prize
60,000 USDC
nSLOC
2,87821
USDC / LOC
50,000 USDC
5,500 USDC
4,500 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.