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

Trades aimed at reducing risk for the position can be blocked due to OpenInterest and Skew checks 

cryptomoon

Summary

In PerpMarketlibrary, function checkOpenInterestLimitsis used to ensure that newly created or settled order doesn't increase open interest or skew when they are decreasing the position. However, in some circumstances, it's possible that reducing the position size reverts due to checks present in this function.

Vulnerability Details

The function checkOpenInterestLimitsis used to ensure that created and settled market order doesn't increase open interest above maxOpenInterest and skew above maxSkew. In the case of open interest more than maxOpenInterest, the checks ensure that the function doesn't revert if the order is decreasing open interest. Also, in the case of skew more than maxSkew, the checks ensure that the function doesn't revert if the order is decreasing the skew. However, this checks shouldn't be present when the account is decreasing their position size. The checks may prevent certain trades from being executed. This could lead to situations where traders are unable to perform trades that would decrease their risk, potentially leading to liquidations. The comments says the following:

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

// 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

However, there can be the case that the order is reducing Open Interestbut increasing the skew. In such cases, traders won't be able to reduce or close their position leading them subjected to liquidation.

Consider following example:

Initial State:

  • Long Positions: 130

  • Short Positions: -30

  • Old Open Interest: 160

  • Old Skew: 100

  • Old max Open Interest: 180

  • Old max skew: 120

  • Trader: -15(short position)

Owner Update:

  • New max skew: 100

  • New max Open Interest: 120

Trade Attempt by trader to close the short position by placing long order of 15. Values after trade settlement:

  • Long Positions: 130

  • Short Positions: -15

  • New Open Interest: 145 (reduced)

  • New Skew: 115 (increased)

Since newOpenInterestis greater than maxOpenInterestand the order is reducing OpenInterest, it should be allowed.

But newSkewis greater that maxSkewand the order is not reducing maxSkew, so this order is not allowed.

This leads to revert of order of closing position of trader.

Occurences in codebase:

In SettlementBranch

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/SettlementBranch.sol#L456-L457

(ctx.newOpenInterestX18, ctx.newSkewX18) =
perpMarket.checkOpenInterestLimits(sizeDeltaX18, ctx.oldPositionSizeX18, ctx.newPositionSizeX18);

In OrderBranch

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/OrderBranch.sol#L305-L307

perpMarket.checkOpenInterestLimits(
ctx.sizeDeltaX18, ctx.positionSizeX18, ctx.positionSizeX18.add(ctx.sizeDeltaX18)
);

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

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());
}
}
}

Impact

Traders may be unable to execute trades that would reduce their overall open interest, potentially leading to forced liquidations.

Tools Used

Manual review

Recommendations

Modify Skew Handling Logic: Update the skew handling logic to allow trades that either reduce open interest or reduce skew. This ensures that trades aimed at reducing risk are not unnecessarily blocked.

By implementing following change, trades that reduce either open interest or skew will be allowed, ensuring that traders can effectively manage their risk and avoid forced liquidations.

if (newSkew.abs().gt(maxSkew)) {
bool isReducingSkew = currentSkew.abs().gt(newSkew.abs());
bool isReducingOI = newOpenInterest.lt(currentOpenInterest);
​
if (!isReducingSkew && !isReducingOI) {
revert Errors.ExceedsSkewLimit(self.id, maxSkew.intoUint256(), newSkew.intoInt256());
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
11 months ago
inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.