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

The order of a liquidatable account can be filled if it was not liquidatable at the time of order creation

fyamf

Summary

If between the creation and filling of an order, the account becomes liquidatable, then the filling would be successful, although the protocol intends to prevent liquidatable accounts from trading.

Vulnerability Details

Increasing/decreasing size in a liquidatable position is not allowed.

{
// get account's current required margin maintenance (before this trade)
(, ctx.previousRequiredMaintenanceMarginUsdX18,) =
tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(0, SD59x18_ZERO);
​
// prevent liquidatable accounts from trading
if (TradingAccount.isLiquidatable(ctx.previousRequiredMaintenanceMarginUsdX18, marginBalanceUsdX18)) {
revert Errors.AccountIsLiquidatable(tradingAccountId);
}
}

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/branches/OrderBranch.sol#L128-L137

But, if a decreasing size order is created, and before it is filled, the account becomes liquidatable, then this order can be filled. This is against the intention of the protocol because decreasing size order is filled even though it is liquidatable.

The root cause of this issue is that during filling the order, it does not check that the associated account is liquidatable or not. It is only checked during creating order in the function simulateTrade.

PoC

In the following test, a long position with size 80 is created and filled. Later, another position with size -50 is created. Before this order is filled, the price of ETH decreases by 8$, making the account liquidatable (marginBalance < requiredMaintenanctMargin). But, since the order is already created, the filling of this order would be done successfully.

function test_becomesLiquidatableBetweenCreationAndFilling()
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));
​
// Creating the first order
perpsEngine.createMarketOrder(
OrderBranch.CreateMarketOrderParams({
tradingAccountId: ctx.tradingAccountId,
marketId: ctx.fuzzMarketConfig.marketId,
sizeDelta: int128(80e18)
})
);
​
// 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);
​
changePrank({ msgSender: users.naruto.account });
​
// Creating the second order
perpsEngine.createMarketOrder(
OrderBranch.CreateMarketOrderParams({
tradingAccountId: ctx.tradingAccountId,
marketId: ctx.fuzzMarketConfig.marketId,
sizeDelta: int128(-50e18)
})
);
​
// Price decreases
updateMockPriceFeed(ctx.fuzzMarketConfig.marketId, (1000 - 8) * 1e18);
​
// Checking the account is liquidatable or not
uint128[] memory liquidatableAccountsIds = perpsEngine.checkLiquidatableAccounts(0, 1);
console.log("account id: ", ctx.tradingAccountId);
console.log("liquidatableAccountsIds: ", liquidatableAccountsIds[0]); // so the account is liquidatable
​
// Filling the second order
// It is exepected that it reverts here, because the account is liquidatable, but it does not
changePrank({ msgSender: ctx.marketOrderKeeper });
ctx.firstMockSignedReport = getMockedSignedReport(ctx.fuzzMarketConfig.streamId, (1000 - 8) * 1e18);
perpsEngine.fillMarketOrder(ctx.tradingAccountId, ctx.fuzzMarketConfig.marketId, ctx.firstMockSignedReport);
}

Impact

  • If an account becomes liquidatable before its order is filled, the order will be filled although it is against the protocol rule.

Tools Used

Recommendations

During filling the order, it should check the liquidatability of the account by calling the function checkLiquidatableAccounts.
https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/branches/LiquidationBranch.sol#L42C14-L42C39

Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Validated
Assigned finding tags:

`isLiquidatable` check missing in `_fillOrder()`

Appeal created

infect3d Auditor
10 months ago
inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

`isLiquidatable` check missing in `_fillOrder()`

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.