Zaros Part 1

Summary

Since accountsIdsWithActivePositions is a large enumerable set, It is impossible to use checkLiquidatableAccounts to definitely find all liquidatable accounts in the set accountsIdsWithActivePositions

Vulnerability Details

The purpose of checkLiquidatableAccounts is to loop through some of the accountsIdsWithActivePositions and determine which accounts are liquidatable.

The reason it can only check some of accountsIdsWithActivePositions at a time is because accountsIdsWithActivePositions can be an extremely large set and since checkLiquidatableAccounts uses a for loop it will run out of gas for large sets.

The issue is that when an account is removed from the set, the last account from the set is moved to the position that was occupied by the removed element. For example if we have 10 elements in index's 1 - 10, if we remove element 5 at index 5, it will be replaced by the last element 10 such that the final state has element 10 at index 5.

Consider the following simplified example

There are 1000 accountsIdsWithActivePositions

1. liquidator calls checkLiquidatableAccounts with lower bound = 0 and upper bound = 500

2. None return liquidatable

3. the account in index 5 closes their position, now they will be removed from the set accountsIdsWithActivePositions and replaced with the last element, in this case account 1000

4. the liquidator calls checkLiquidatableAccounts lower bound = 500 and upper bound = 1000

5. None return liquidatable

6. At this point the liquidator thinks that no accounts are liquidatable BUT they did not check account 1000 since it is now in index 5

The scenario only uses 1000 accounts for understanding sake. In reality the protocol is designed to support millions of accounts, that would magnify the impact of this issue

Impact

Liquidatable accounts will be missed and not liquidated

Tools Used

Manual Review

Recommendations

Do not use the enumerable set when it can get large very easily and you need to check every single element, for example in liquidations