Summary

The checkUpkeep function in LiquidationKeeper contains a potential index manipulation vulnerability that could lead to account liquidation bypass or unauthorized liquidations.

Vulnerability Details

In teh checkUpkeep function :

The vulnerability lies in:

1. No validation of performLowerBound against liquidatableAccountsIds.length

2. Potential integer overflow in performLowerBound + i

3. Possible array access manipulation through carefully crafted bounds

Impact

• Bypass of liquidation queue order

• Potential skip of accounts that should be liquidated

• Manipulation of liquidation priority

• Possible DOS attack by setting performLowerBound to a very large number

Tools Used

• Manual code review

• Slither

• Mythril

• Custom fuzzing tests

Recommendations

1. Add proper bounds validation and array access protection:

   /Users/jojo/Library/Mobile Documents/com~apple~CloudDocs/AUDIT/2025-01-zaros-part-2/src/external/chainlink/keepers/liquidation/LiquidationKeeper.sol

   ​

   function checkUpkeep(bytes calldata checkData)

   external

   view

   returns (bool upkeepNeeded, bytes memory performData)

   {

   (uint256 checkLowerBound, uint256 checkUpperBound, uint256 performLowerBound, uint256 performUpperBound) =

   abi.decode(checkData, (uint256, uint256, uint256, uint256));

   ​

   // Add validation for performLowerBound

   if (performLowerBound >= liquidatableAccountsIds.length) {

   revert Errors.InvalidPerformBound();

   }

   ​

   // Protect against overflow

   unchecked {

   if (performLowerBound + performLength > liquidatableAccountsIds.length) {

   performLength = liquidatableAccountsIds.length - performLowerBound;

   }

   }

   ​

   for (uint256 i; i < performLength; i++) {

   uint256 accountIdIndex;

   unchecked {

   accountIdIndex = performLowerBound + i;

   }

   require(accountIdIndex < liquidatableAccountsIds.length, "Index out of bounds");

   accountsToBeLiquidated[i] = liquidatableAccountsIds[accountIdIndex];

   if (!upkeepNeeded && liquidatableAccountsIds[accountIdIndex] != 0) {

   upkeepNeeded = true;

   }

   emit AccountProcessed(accountIdIndex, accountsToBeLiquidated[i]);

   }

   }

   ​

   event AccountProcessed(uint256 indexed index, uint128 accountId);

2/ Add additional safety checks:

3/ Implement a sequential processing mechanism:

• Track the last processed index

• Enforce sequential processing of accounts

• Add mechanisms to handle priority liquidations

These changes will significantly improve the security and reliability of the liquidation process while preventing potential manipulation of the account processing order.