Zaros — LiquidationBranch::checkLiquidatableAccounts() array out-of-bounds

Source & credit. Reproduction of a public audit finding curated by AuditVault — the original finding: 37994-liquidationbranchcheckliquidatableaccounts-executes-for-loop. Standalone Foundry PoC and full write-up: 37994-liquidationbranchcheckliquidatableaccounts-executes-for-loop_exp in the evm-hack-registry mirror.

Vulnerability classes: vuln/logic/off-by-index · vuln/liveness/dos · vuln/integer-bounds/array-out-of-bounds

Reproduction: a self-contained Foundry PoC that compiles & runs in an isolated project with only forge-std — no fork, no RPC, no anvil_state. Full trace: output.txt. PoC: test/37994-liquidationbranchcheckliquidatableaccounts-executes-for-loop_exp.sol.

Key info#

This is an audit finding, not a historical on-chain incident. The real checkLiquidatableAccounts reads live margin/PnL data for each active trading account; the PoC keeps the vulnerable array-sizing/indexing logic verbatim and replaces the margin computation with a simple per-index liquidatable flag, reduced to the minimum needed to trigger the exact out-of-bounds condition.

TL;DR#

1. checkLiquidatableAccounts(lowerBound, upperBound) is explicitly designed for segmented/paginated scanning of active trading accounts (per its own NatSpec @param docs) — the output array is sized to the segment width, upperBound - lowerBound.
2. The scan loop walks the absolute range for (i = lowerBound; i < upperBound; i++) and writes results at liquidatableAccountsIds[i] — using the absolute i, not the segment-relative offset i - lowerBound.
3. The moment lowerBound != 0 and the loop reaches a liquidatable account, i is already >= the array's length (upperBound - lowerBound), and the write reverts with an array out-of-bounds Panic(uint256, 0x32).
4. Harm: every segment except the very first (lowerBound == 0) is completely non-functional — the entire call reverts, returning nothing, not even the accounts before the first liquidatable one in the segment. This is consumed by LiquidationKeeper.checkUpkeep (Chainlink Automation), so paginated liquidation monitoring silently breaks for any segment beyond the first, risking undercollateralized positions going unliquidated.

The vulnerable code#

LiquidationBranch.sol::checkLiquidatableAccounts (verbatim):

/// @param lowerBound The lower bound of the accounts to check
/// @param upperBound The upper bound of the accounts to check
function checkLiquidatableAccounts(
    uint256 lowerBound,
    uint256 upperBound
)
    external
    view
    returns (uint128[] memory liquidatableAccountsIds)
{
    // prepare output array size
    liquidatableAccountsIds = new uint128[](https://github.com/sanbir/evm-hack-registry/tree/main/37994-liquidationbranchcheckliquidatableaccounts-executes-for-loop_exp/upperBound%20-%20lowerBound);
    ...
    for (uint256 i = lowerBound; i < upperBound; i++) {
        if (i >= cachedAccountsIdsWithActivePositionsLength) break;
        uint128 tradingAccountId = uint128(globalConfiguration.accountsIdsWithActivePositions.at(i));
        ...
        if (TradingAccount.isLiquidatable(requiredMaintenanceMarginUsdX18, marginBalanceUsdX18)) {
            liquidatableAccountsIds[i] = tradingAccountId;
        }
    }
}

The fix, per the finding (indexing with a running count instead of the absolute i, then resizing to drop trailing zeroes):

+       uint256 count = 0;
        for (uint256 i = lowerBound; i < upperBound; i++) {
            ...
            if (TradingAccount.isLiquidatable(requiredMaintenanceMarginUsdX18, marginBalanceUsdX18)) {
-               liquidatableAccountsIds[i] = tradingAccountId;
+               cacheLiquidatableAccountsIds[count] = tradingAccountId;
+               count++;
            }
        }
+       liquidatableAccountsIds = new uint128[](https://github.com/sanbir/evm-hack-registry/tree/main/37994-liquidationbranchcheckliquidatableaccounts-executes-for-loop_exp/count);
+       for (uint256 j = 0; j < count; j++) { liquidatableAccountsIds[j] = cacheLiquidatableAccountsIds[j]; }

Root cause#

The output array's length is derived from the segment width (upperBound - lowerBound), but the loop's write index is derived from the absolute scan position (i). These two quantities only coincide when lowerBound == 0. For any other segment, i starts already at or beyond the array's length, so the first liquidatable account found in that segment triggers an out-of-bounds write.

Preconditions#

• The caller (typically a keeper contract, e.g. Chainlink Automation via LiquidationKeeper.checkUpkeep) calls checkLiquidatableAccounts with a non-zero lowerBound — exactly the intended usage pattern for segmenting a large active-account set into gas-bounded chunks.
• At least one account within [lowerBound, upperBound) is liquidatable.

Attack walkthrough#

From output.txt, mirroring the finding's own PoC scenario (30 active accounts, indices 10..19 liquidatable, lowerBound=10, upperBound=20):

1. Control — checkLiquidatableAccounts(0, 10) returns a correctly-sized 10-element array. lowerBound == 0, so the absolute index and the relative offset coincide — no bug visible.
2. HARM — checkLiquidatableAccounts(10, 20) is called (via a raw staticcall in the PoC so the revert can be observed without aborting the whole PoC, exactly as a keeper's checkUpkeep would observe it).
3. At i = 10 — the very first index checked in this segment, which is marked liquidatable — the write to liquidatableAccountsIds[10] overflows the 10-slot array.
4. The call reverts with Panic(uint256), code 0x32 (array out-of-bounds access) — the exact failure the finding's own PoC reproduces (vm.expectRevert(abi.encodeWithSignature("Panic(uint256)", 0x32))).
5. The ENTIRE segment call fails — not just the one liquidatable account, but every account in [10, 20), liquidatable or not, produces nothing.

Diagrams#

Impact#

• Every paginated liquidation-scan segment except the first (lowerBound == 0) is entirely non-functional whenever it contains a liquidatable account — it reverts instead of returning results.
• LiquidationKeeper.checkUpkeep consumes this function's output to decide which accounts to liquidate; a reverting segment means Chainlink Automation either fails the upkeep check entirely for that segment or (depending on how the revert propagates through the keeper) silently sees no liquidatable accounts there.
• The practical consequence is that undercollateralized positions in any segment beyond the first can go unliquidated, exposing the protocol to accumulating bad debt exactly during the periods (many active accounts, segmentation actually needed) where liquidation monitoring matters most.

Remediation#

Track a separate running counter for the number of liquidatable accounts found, write into the output array at that counter (not at the absolute loop index i), and resize the array to the counter's final value before returning — exactly as shown in the finding's diff above.

How to reproduce#

cd ~/RustroverProjects/audits/evm-hack-registry/37994-liquidationbranchcheckliquidatableaccounts-executes-for-loop_exp
forge test -vvv
# Fully local — no fork, no RPC, no anvil_state required.
# Expected: all three tests PASS:
#   test_lowerBoundZero_worksCorrectly         (control: first segment works)
#   test_lowerBoundNonZero_revertsOutOfBounds  (harm, via vm.expectRevert: Panic(0x32))
#   test_exploit_demonstratesOutOfBoundsRevert (harm, via the cheatcode-free Exploit + staticcall)

PoC source: test/37994-liquidationbranchcheckliquidatableaccounts-executes-for-loop_exp.sol — the verbatim vulnerable array-sizing and absolute-indexing lines, reduced to a minimal active-account list with a per-index liquidatable flag.

Sources#

• AuditVault finding: 37994-liquidationbranchcheckliquidatableaccounts-executes-for-loop.md
• Original contest: Codehawks 2024-07-zaros (Cyfrin), finding #37994
• Reduced source: quoted directly from the finding (LiquidationBranch::checkLiquidatableAccounts), repo Cyfrin/2024-07-zaros@d687fe9

Taxonomy (AuditVault)#

• genome: liquidation-logic, permanent, chainlink-round-completeness, integer-bounds, liquidation-underwater
• sector: lending, oracle, perpetuals
• severity: high

Reference: finding #37994 by cryptedOji in the Codehawks 2024-07-zaros contest (Cyfrin) · curated by AuditVault

Sources & further analysis#

Reproductions & code

• Standalone PoC + full trace: 37994-liquidationbranchcheckliquidatableaccounts-executes-for-loop_exp (evm-hack-registry mirror).
• AuditVault finding: 37994-liquidationbranchcheckliquidatableaccounts-executes-for-loop.

Alerts & third-party analyses

• Web3Sec X hacked database: search.
• Rekt leaderboard: search.
• Solodit incident search: search.

These dashboards index community alerts tweets, post-mortems, and independent write-ups. Reach them through the protocol name above to cross-check this reproduction against other analyses.