Zaros Part 1

Summary

Vulnerability Details

The checkLiquidatableAccounts function in the LiquidationBranch.sol is designed to scan a range of trading accounts and identify which ones are liquidatable. It takes two parameters, lowerBound and upperBound, to define the range of accounts to check. The function is intended to return an array of liquidatableAccountsIds that are eligible for liquidation.

The liquidatableAccountsIds array is correctly initialized with a size equal to the range being checked. The loop iterates from lowerBound to upperBound, checking each account.

The bug occurs in the array assignment:

The index i is used directly to store the liquidatable account ID. The core issue is the mismatch between the loop variable i, which starts from lowerBound, and the indexing of liquidatableAccountsIds, which starts at 0. This mismatch leads to reverts due to the out-of-bounds write.

The function will fail to return any liquidatable accounts, even though there are liquidatable accounts in the given range. This could prevent necessary liquidations from happening, potentially affecting the stability of the system.

Simple Scenario:

1. Let's say lowerBound = 5, upperBound = 10, and cachedAccountsIdsWithActivePositionsLength = 8.

2. The loop will iterate from 5 to 7 (inclusive).

3. For any liquidatable accounts found, it will try to write to liquidatableAccountsIds[5], liquidatableAccountsIds[6], or liquidatableAccountsIds[7].

4. But liquidatableAccountsIds was initialized with length 5 (10 - 5), so indices 5, 6, and 7 are out of bounds.

Impact

Liquidation functionality of the protocol is broken. Missed liquidations can result in the protocol holding undercollateralized positions for longer than intended. This increases the protocol's risk exposure, potentially leading to larger losses if market conditions deteriorate.

Tools Used

Manual Review

Recommendations

To fix this, you should use a separate counter for the array index: