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

Improper position checking allows the trader to bypass certain checks and apply a maitenance margin requirement instead of the initial margin requirement

Summary

When an order is created, the Position.isIncreasing function is called to determine whether the trader is trying to increase or decrease the position.

Data storage self = Position.load(tradingAccountId, marketId);
return self.size == 0 || (self.size > 0 && sizeDelta > 0) || (self.size < 0 && sizeDelta < 0);

This code above is the body of mentioned function. It assumes that:

  • long position is increasing when sizeDelta is positive

  • short position is increasting when sizeDelta is negative

Vulnerability Details

The trader can cheat mentioned functions by performing the following steps

  • Trader opens short position with sizeDelta = -1

  • Trader opens long position with sizeDelta = 10001

Because his current size is less than 0 and sizeDelta is positive, that function says this position is decreasing which is not true.

Impact

1: Position increasing is allowed only when both market and settlement are enabled. If not, the protocol only allows to close and decrease positions. Traders would not be able to use this hack to open new position ( they would not be able to open new position even with sizeDelta - 1 because that is increasing ), but nothing would stop them from increasting already existing positions. They would only need to flip the skew, by first opening a new order with the opposite size sign + 1 ( short => long / long => short), and then do this again.

if (ctx.isIncreasing) {
globalConfiguration.checkMarketIsEnabled(marketId);
settlementConfiguration.checkIsSettlementEnabled();
}

2: Using this method they can use maitenance margin requirement instead of initial margin requirement which is lower.

ctx.shouldUseMaintenanceMargin = !ctx.isIncreasing && !ctx.oldPositionSizeX18.isZero();
ctx.requiredMarginUsdX18 = ctx.shouldUseMaintenanceMargin ? requiredMaintenanceMarginUsdX18 : requiredInitialMarginUsdX18;

In both mentioned cases, it is assumed that the trader is doing this on porpus, which may not always be true, he might not know about it. In such case, he might end up exposured for bigger liqudation risk, or participate in disabled markets

Tools Used

Manual Review

Recommendations

Position.isIncreasing could compare absolute value of self.size + sizeDelta to absolute value of selfSize to say if it is increasing or not

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!