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

Incorrect handling of open-interest and skew update in liquidation function will reset open interest and skew to zero instead of reducing it values.

bigsam

Summary

The current implementation of the liquidation function incorrectly resets the open interest and skew to zero, which are crucial indicators of the amount of long and short positions in the market. This flaw occurs because the `updateOpenInterest` function is called with default values that have not been properly set, leading to the incorrect updating of these indicators during liquidation.

Vulnerability Details

The issue lies in the implementation of the liquidation function, which incorrectly sets the open interest and skew to zero. This happens because the `updateOpenInterest` function is called with `ctx.newOpenInterestX18` and `ctx.newSkewX18`, which have not been properly initialized and hold their initial values of zero. The developers' comments indicate that open interest and skew should be decreased by liquidations, and there should be checks to prevent Denial of Service (DoS) attacks in case those cap checks fail. However, these checks are not properly implemented.

### Key Points:

1. **Current Behavior**:

- Open interest and skew are reset to zero during liquidation due to uninitialized values.

- The function `updateOpenInterest` is called without ensuring the values are correctly set.

2. **Developers' Intentions**: Comment in code

- Open interest and skew should be decreased during liquidation.

- The system should allow a reduction in open interest regardless of the maximum open interest limit to prevent DoS attacks.

- Proper checks should be in place to ensure open interest and skew are correctly updated.

// update perp market's open interest and skew; we don't enforce open
// 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);

From the above we can see that the developer didn't call CheckOpenInterestLimit first.

/// @notice Verifies the market's open interest and skew limits based on the next state.
/// @dev During liquidation we skip the max skew check, so the engine can always liquidate unhealthy accounts.
/// @dev If the case outlined above happens and the maxSkew is crossed, the market will only allow orders that
/// reduce the skew.
/// @param self The PerpMarket storage pointer.
/// @param sizeDelta The size delta of the order.
/// @param oldPositionSize The old position size.
/// @param newPositionSize The new position size.
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());
​
// if new open interest would be greater than this market's max open interest,
// we still want to allow trades as long as they decrease the open interest. This
// allows traders to reduce/close their positions in markets where protocol admins
// have reduced the max open interest to reduce the protocol's exposure to a given
// perp market
if (newOpenInterest.gt(maxOpenInterest)) {
// is the proposed trade reducing open interest?
bool isReducingOpenInterest = currentOpenInterest.gt(newOpenInterest);
​
// revert if the proposed trade isn't reducing open interest
if (!isReducingOpenInterest) {
revert Errors.ExceedsOpenInterestLimit(
self.id, maxOpenInterest.intoUint256(), newOpenInterest.intoUint256()
);
}
}
​
// load max & current skew for this perp market uint128 -> UD60x18 -> SD59x18
SD59x18 maxSkew = ud60x18(self.configuration.maxSkew).intoSD59x18();
// int128 -> SD59x18
SD59x18 currentSkew = sd59x18(self.skew);
​
// calculate new skew
newSkew = currentSkew.add(sizeDelta);
​
// similar logic to the open interest check; if the new skew is greater than
// the max, we still want to allow trades as long as they decrease the skew
if (newSkew.abs().gt(maxSkew)) {
bool isReducingSkew = currentSkew.abs().gt(newSkew.abs());
​
if (!isReducingSkew) {
revert Errors.ExceedsSkewLimit(self.id, maxSkew.intoUint256(), newSkew.intoInt256());
}
}
}

Not calling the above leave newopen interest and skew at zero. check implementation in market order also

Impact

Resetting open interest and skew to zero during liquidation. This could result in a significant misrepresentation of the market’s long and short positions.

Tools Used

- Manual Solidity code analysis

- Developer comments review

Recommendations

/// @param accountsIds The list of accounts to liquidate
function liquidateAccounts(uint128[] calldata accountsIds) external {
-------------------------------------------------------------------
// 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.checkOpenInterestLimits(ctx.liquidationSizeX18, ctx.oldPositionSizeX18, SD59x18_ZERO);
perpMarket.updateOpenInterest(ctx.newOpenInterestX18, ctx.newSkewX18);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months 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.