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

Disable market limitation can be bypassed

jennifersun

Summary

When one market is disabled, the allowed behavior for this market is closing position or decreasing position. This limitation can by bypassed.

Vulnerability Details

When one market is disabled, the allowed behavior for this market is closing position or decreasing position. The contract has one isIncreasing check. If this order will increase the position, will not be allowed.
However, the vulnerability is that isIncreasing() does not include all possible increasing position's scanraios.
For example:

  • Alices open one Long position with size 10 in market A.

  • The owner disables this markets.

  • If Alice wants one Short position, she can open one order with SHORT position with size -20 in market A. Then Alice will create one Short position with size -10.

  • Even if Alice want to enlarge her LONG position, Alice can create one SHORT position with size -20 in market A. This will cause self.size < 0. And then create one LONG position with size +100 in market A. This will enlarge her LONG position.

function createMarketOrder(CreateMarketOrderParams calldata params) external {
...
// determine whether position is being increased or not
ctx.isIncreasing = Position.isIncreasing(params.tradingAccountId, params.marketId, params.sizeDelta);
​
// both markets and settlement can be disabled, however when this happens we want to:
// 1) allow open positions not subject to liquidation to decrease their size or close
// 2) prevent new positions from being opened & existing positions being increased
//
// the idea is to prevent a state where traders have open positions but are unable
// to reduce size or close even though they can still be liquidated; such a state
// would severly disadvantage traders
if (ctx.isIncreasing) {
// both checks revert if disabled
globalConfiguration.checkMarketIsEnabled(params.marketId);
settlementConfiguration.checkIsSettlementEnabled();
}
function isIncreasing(
uint128 tradingAccountId,
uint128 marketId,
int128 sizeDelta
)
internal
view
returns (bool)
{
Data storage self = Position.load(tradingAccountId, marketId);
​
// 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.
// No existing position or delta is in the same direction with the previous position.
return self.size == 0 || (self.size > 0 && sizeDelta > 0) || (self.size < 0 && sizeDelta < 0);
}

Impact

Traders can bypass the disabled market limitation and go on trading on the disabled market.

Tools Used

Manual

Recommendations

If one market is disabled, the deltaSize's abs is not allowed to be larger than the previous position's size's abs.

Updates

Lead Judging Commences

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