Zaros Part 1

Summary

In the `LiquidationBranch::checkLiquidatableAccounts` function, we observe an iteration over active accounts within specified bounds, checking whether each account is liquidatable. According to the for loop, the index `i` starts from `lowerBound` and ends at `upperBound`:

However, in the last line of the for loop, the `LiquidationBranch::liquidatableAccountsIds` array is initialized as follows:

This indicates that if `lowerBound` is equal to zero, everything functions correctly, as array initialization begins at index zero and ends with the value of `upperBound`. In contrast, if `lowerBound` is greater than zero, array initialization also begins from an index greater than zero. Consequently, it is possible for the function to attempt to initialize indices that exceed the length of the array, which is determined by the difference between `upperBound` and `lowerBound`. This cause the `LiquidationBranch::checkLiquidatableAccounts` function to revert.

Vulnerability Details

In a case that `lowerBound` is more than zero, Let's assume:

lowerBound = 20,

upperBound = 100

So we can define L as:

L = upperBound - lowerBound = 100 - 20 = 80

Also we know the length of `liquidatableAccountsIds` array is equal to L which is 80 in our assumption, starts from index 0 to index 79

So clearly indeces more than 79 is out of array's scope.

Now as an example if account with the index of `90` is liquidable we have liquidatableAccountsIds[90] which lead `checkLiquidatableAccounts` function to revert.

Impact

The `LiquidationBranch::checkLiquidatableAccounts` function always reverts if the `lowerBound` parameter is not zero. As a result, protocol users must search through all accounts to identify liquidatable ones. If the `lowerBound` is intended to always be zero, then including it as a function input is unnecessary.

Tools Used

Manual review

Recommendations

With substracting `i` from `lowerBound` we can shift the index to zero: