Summary
When there are large amount number of accounts, liquidater calls LiquidationBranch.sol#checkLiquidatableAccounts function with lowerBound > 0 to get liguidatable accounts.
But since the function has logical error when lowerBound > 0, the function call will be reverted.
Vulnerability Details
LiquidationBranch.sol#checkLiquidatableAccounts function is the following.
function checkLiquidatableAccounts(
uint256 lowerBound,
uint256 upperBound
)
external
view
returns (uint128[] memory liquidatableAccountsIds)
{
51: liquidatableAccountsIds = new uint128[](upperBound - lowerBound);
if (liquidatableAccountsIds.length == 0) return liquidatableAccountsIds;
GlobalConfiguration.Data storage globalConfiguration = GlobalConfiguration.load();
uint256 cachedAccountsIdsWithActivePositionsLength =
globalConfiguration.accountsIdsWithActivePositions.length();
64: for (uint256 i = lowerBound; i < upperBound; i++) {
if (i >= cachedAccountsIdsWithActivePositionsLength) break;
uint128 tradingAccountId = uint128(globalConfiguration.accountsIdsWithActivePositions.at(i));
TradingAccount.Data storage tradingAccount = TradingAccount.loadExisting(tradingAccountId);
(, UD60x18 requiredMaintenanceMarginUsdX18, SD59x18 accountTotalUnrealizedPnlUsdX18) =
tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(0, SD59x18_ZERO);
SD59x18 marginBalanceUsdX18 = tradingAccount.getMarginBalanceUsd(accountTotalUnrealizedPnlUsdX18);
82: if (TradingAccount.isLiquidatable(requiredMaintenanceMarginUsdX18, marginBalanceUsdX18)) {
83: liquidatableAccountsIds[i] = tradingAccountId;
}
}
}
Example:
Assume that lowerBound = 60 and upperBound = 100.
Then, it will be liquidatableAccountsIds.length = 100 - 60 = 40 in L51.
The iterator i loops from i = 60 to i = 100 in L64.
If an trading account is liguidatable for certain i (60 to 100), the L83 will cause panic error with array out-of-bounds.
PoC:
Modify the checkLiquidatableAccounts.t.sol#testFuzz_WhenThereAreOneOrManyLiquidatableAccounts function as follows.
function testFuzz_WhenThereAreOneOrManyLiquidatableAccounts(
uint256 marketId,
bool isLong,
uint256 amountOfTradingAccounts
)
external
{
MarketConfig memory fuzzMarketConfig = getFuzzMarketConfig(marketId);
amountOfTradingAccounts = bound({ x: amountOfTradingAccounts, min: 1, max: 10 });
uint256 marginValueUsd = 10_000e18 / amountOfTradingAccounts;
uint256 initialMarginRate = fuzzMarketConfig.imr;
deal({ token: address(usdz), to: users.naruto.account, give: marginValueUsd });
for (uint256 i; i < amountOfTradingAccounts; i++) {
uint256 accountMarginValueUsd = marginValueUsd / amountOfTradingAccounts;
uint128 tradingAccountId = createAccountAndDeposit(accountMarginValueUsd, address(usdz));
openPosition(fuzzMarketConfig, tradingAccountId, initialMarginRate, accountMarginValueUsd, isLong);
}
setAccountsAsLiquidatable(fuzzMarketConfig, isLong);
-- uint256 lowerBound = 0;
uint256 upperBound = amountOfTradingAccounts;
++ uint256 lowerBound = upperBound / 2;
uint128[] memory liquidatableAccountIds = perpsEngine.checkLiquidatableAccounts(lowerBound, upperBound);
assertEq(liquidatableAccountIds.length, amountOfTradingAccounts);
for (uint256 i; i < liquidatableAccountIds.length; i++) {
assertEq(liquidatableAccountIds[i], i + 1);
}
}
The result is the following
Failing tests:
Encountered 1 failing test in test/integration/perpetuals/liquidation-branch/checkLiquidatableAccounts/checkLiquidatableAccounts.t.sol:CheckLiquidatableAccounts_Integration_Test
[FAIL. Reason: panic: array out-of-bounds access (0x32); counterexample: calldata=0xcac72dcd00000000000000000000000000000000000000000000000000000000000041a00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000162d args=[16800 [1.68e4], false, 5677]] testFuzz_WhenThereAreOneOrManyLiquidatableAccounts(uint256,bool,uint256) (runs: 0, μ: 0, ~: 0)
Impact
When there are large number of accounts, The liquidator should call LiquidationBranch.sol#checkLiquidatableAccounts function with lowerBound > 0 to avoid huge gas fee.
Then since the call will be reverted, the liquidator can't liquidate liquidatable accounts in time and it may cause the protocol unsolvent.
Tools Used
Manual Review
Recommendations
Modify the LiquidationBranch.sol#checkLiquidatableAccounts function as follows.
function checkLiquidatableAccounts(
uint256 lowerBound,
uint256 upperBound
)
external
view
returns (uint128[] memory liquidatableAccountsIds)
{
... SKIP ...
if (TradingAccount.isLiquidatable(requiredMaintenanceMarginUsdX18, marginBalanceUsdX18)) {
-- liquidatableAccountsIds[i] = tradingAccountId;
++ liquidatableAccountsIds[i - lowerBound] = tradingAccountId;
}
}
}
