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

Incorrect check allows users to increase their positions even when a market is disabled

samuraii77

Summary

Incorrect check allows users to increase their positions even when a market is disabled

Vulnerability Details

If a user is increasing his position whenever a market is disabled or whenever a settlement is disabled, he will not be allowed to do so due to this check:

if (ctx.isIncreasing) {
// both checks revert if disabled
globalConfiguration.checkMarketIsEnabled(params.marketId);
settlementConfiguration.checkIsSettlementEnabled();
}

This is how isIncreasing gets its value:

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.
return self.size == 0 || (self.size > 0 && sizeDelta > 0) || (self.size < 0 && sizeDelta < 0);
}

A position is increased in these 3 scenarios:

  1. The size of the position was 0 which means that it always increases as there was not a position beforehand

  2. The size of the position was over 0 (long) and he is putting more money into the long (sizeDelta is positive

  3. The size of the position was below 0 (short) and he is putting more money into the short (sizeDelta is negative)

However, this doesn't cover the case where the user had a long position and he is increasing it in the opposite direction. Take this example:

  1. Bob has a position size of 10

  2. He creates a new market order with a sizeDelta of -30

  3. His new position size is -20 which means he increased it but changed it from a long to a short

The bigger issue here is that not only can the user do that but he can also create a new order afterwards and change his position to his initial direction.
Imagine the scenario where the markets are disabled and Bob wants to increase his position from 50 to 100 but he is not allowed to do so due to the check. He can still do so because of the issue above:

  1. Bob has a position size of 50 and he changes it to any negative value (short position)

  2. This passes as self.size is equal 50 and his sizeDelta is equal -100, for example

  3. Now he has a position of -50 and he can change his position to 100 by specifying a sizeDelta of 150

  4. This will pass as self.size will equal -50 and his sizeDelta is equal to 150, self.size is < 0 but sizeDelta is not < 0

  5. Now, he increased his position even though the market was actually disabled

Furthermore, the isIncreasing value is used to determine whether to use the required maintenance margin or the required initial margin which could cause other huge issues like incorrect liquidations.

Impact

Incorrect check allows users to increase their positions even when a market is disabled

Tools Used

Manual Review

Recommendations

Refactor the code to return the proper value. Use the abs() function like you do in many places in the code to properly check whether the position is increasing or decreasing.

Updates

Lead Judging Commences

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