Zaros Part 1

Summary

It is impossible to for the keeper to liquidate an account in certain scenarios

Vulnerability Detail

The keeper must call the function checkLiquidatableAccounts, in order to get a list of accounts that need to be liquidated in the checkUpkeep function. The keeper must input a lower and upper bound of accounts to check, this ensures we can check accounts in increments to avoid oog errors.

The problem occurs because of an error in a for loop in the checkLiquidatableAccounts function. The for loop is used to add accounts which need to be liquidated into the LiquidatableAccountIds array which is used by the keeper to liquidate accounts.

Above is the loop in question.
cachedAccountsIdsWithActivePositionsLength is the total amount of account ids with active positions.
If we assume the lower bound is 10 and the uppper bound is 20, because there can be account ids without active positions, the cachedAccountsIdsWithActivePositionsLength may be 1.

Assuming this configuration, the if statement will cause a break and the account id will not be checked for its liquidation status and thus not liquidated by the keeper.

Impact

Accounts which must be liquidated will not be able to liquidated by the keeper, causing bad debt to increase in the protocol.

Code Snippet

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/LiquidationBranch.sol#L64

Tool used

Manual Review

Recommendation

scrap the first if statement or change it to better handle the scenario described above.