DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: high
Valid

The `isIncreasing` check can be bypassed by flipping a position, allowing a position to be opened with less margin and bypassing market closure restrictions.

Summary

The isIncreasing function assumes the position is increasing if the size and delta move in the same direction. However, when a user makes a change where the delta is larger than the size in the opposite direction, then the existing position is closed AND a new position is created.

The check will wrongly consider the position as decreasing, return false and not pass the checkMarketIsEnabled and checkIsSettlementEnabled checks.

This allows a user to create a position with less margin than normally required (maintenanceMargin < initialMargin ) and create/fill an order in a market that is not enabled.

Vulnerability Details

  1. The isIncreasing function is used to determine whether the user's order is an increase in position when creating an order or executing an order.

  2. If the user is adding to a position, two checks must be performed. checkMarketIsEnabled and checkIsSettlementEnabled. This ensures that the market is active at this time.

  3. If the user is increasing a position, they need to ensure that they have sufficient initial margin, otherwise they only need to ensure that they have sufficient maintenance margin. Initial margin is generally larger than maintenance margin.

But in the isIncreasing function, there is a situation that is not taken into account.
Assume that user A currently has a position of -100, and the user creates an order to increase the position by 300. After the increase, the user's position is 200.

// If position is being opened (size is 0) or if position is being increased (sizeDelta direction is same as
// position size)
// For a positive position, sizeDelta should be positive to increase, and for a negative position, sizeDelta
// should be negative to increase.
// This also implicitly covers the case where if it's a new position (size is 0), it's considered an increase.
return self.size == 0 || (self.size > 0 && sizeDelta > 0) || (self.size < 0 && sizeDelta < 0);

In the corresponding code, self.size==-100, sizeDelta==200. Obviously, false will be returned here, which means that protocol consideres the user as not adding a position. But in this case, the user clearly increased the position to 200.

Therefore, for the attacker, the user can create the minimum reverse position, such as -1, and then create the position he wants to create, such as 200, and then only pay less margin to add the position.
And he can bypass both of the above market checks. Adding positions when the market is not active.

Because users can open positions without sufficient margin, I judge the impact to be high. No external conditions are required, so the possibility is also high.

Impact

  1. An attacker can open a position by paying less margin.

  2. An attacker can open a position by bypassing the market's enable restrictions.

Tools Used

Manual Review

Recommendations

Additional logic in the isIncreasing function needs to provided to account for the case where delta > size.

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Disable market limitation can be bypassed

Support

FAQs

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

Give us feedback!