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

It is possible to increase a position when neither the market nor the settlement are enabled. Also during settlement, the margin required to back the position will no be the highest

draiakoo

Summary

It is possible to increase a position when neither the market nor the settlement are enabled. Also during settlement, the margin required to back the position will no be the highest

Vulnerability Details

When someone creates a market order, there is a check to ensure that if the market or the settlement are not enabled it is not possible to increase the size of the position. So essentially the user would only be able to decrease it. This check is implemented as follows:

ctx.isIncreasing = Position.isIncreasing(params.tradingAccountId, params.marketId, params.sizeDelta);
​
if (ctx.isIncreasing) {
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.
return self.size == 0 || (self.size > 0 && sizeDelta > 0) || (self.size < 0 && sizeDelta < 0);
}

Looking at what is considered increasing a position we can see that it only handle the cases where the size is increase in the same direction of the already active size and when a position had no previous size. For example, if the current size is -300 and the delta size is -100 it is increasing in the same direction and it is considered an increase. However, this function is not taking into account the direction swapping. If a user had a short position of -100 and now wants to long the token by +1000, he will get a position of +900 which has increased the open interest and in the coded implementation has not considered it as an increase, so that means that he has bypassed these 2 security checks for the market and the settlement enabled.
Also during the settlement, the selected margin to be required to back the position choose the least one because it considered that the position is being decreased.

function _fillOrder(
uint128 tradingAccountId,
uint128 marketId,
uint128 settlementConfigurationId,
SD59x18 sizeDeltaX18,
UD60x18 fillPriceX18
)
internal
virtual
{
...
​
// determine whether position is being increased or not
ctx.isIncreasing = Position.isIncreasing(tradingAccountId, marketId, sizeDeltaX18.intoInt256().toInt128());
​
// 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(marketId);
settlementConfiguration.checkIsSettlementEnabled();
}
​
...
{
// calculate required initial & maintenance margin for this trade
// and account's unrealized PNL
(
UD60x18 requiredInitialMarginUsdX18,
UD60x18 requiredMaintenanceMarginUsdX18,
SD59x18 accountTotalUnrealizedPnlUsdX18
) = tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(marketId, sizeDeltaX18);
​
// check maintenance margin if:
// 1) position is not increasing AND
// 2) existing position is being decreased in size
//
// when a position is under the higher initial margin requirement but over the
// lower maintenance margin requirement, we want to allow the trader to decrease
// their losing position size before they become subject to liquidation
//
// but if the trader is opening a new position or increasing the size
// of their existing position we want to ensure they satisfy the higher
// initial margin requirement
ctx.shouldUseMaintenanceMargin = !ctx.isIncreasing && !ctx.oldPositionSizeX18.isZero();
​
ctx.requiredMarginUsdX18 =
ctx.shouldUseMaintenanceMargin ? requiredMaintenanceMarginUsdX18 : requiredInitialMarginUsdX18;
​
// reverts if the trader can't satisfy the appropriate margin requirement
tradingAccount.validateMarginRequirement(
ctx.requiredMarginUsdX18,
tradingAccount.getMarginBalanceUsd(accountTotalUnrealizedPnlUsdX18),
ctx.orderFeeUsdX18.add(ctx.settlementFeeUsdX18)
);
}
​
...
}

As we can see, the validation for the margin requirement will always select the maintenance margin which is always smaller than the initial maintenance margin, that is because the ctx.shouldUseMaintenanceMargin will be true. So the result is that the user will need less margin than it would to if it was considered an increase.

It is true that if a user wants to exploit this issue, he needs to have an active position reversed before the market or the settlement gets disabled. That is because opening a position when you did not have any size it will be considered an increase and it will revert due to the market disabled. For example, if a user wants to increase a long position, he will need to have an active short position before, and if he wants to increase a short position, he will need a long position before.
The user can just open a position for the maximum amount of markets with a little size and wait when the market gets disabled, then he can execute this issue.

Impact

Medium, the open interest will increase even when the market or the settlement is disabled

Tools Used

Manual review

Recommendations

I would not consider an increase using these 3 conditions, I would use instead the change of the open interest once the trade is accounted. If the open interest increases compared to the previous one it will be 100% an increase of a position. If it does not increase it means that the position is decreasing his size.

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.