Zaros Part 1

Vulnerability Details

In the LiquidationKeeper.sol contract, there is a critical mismatch between the data encoding in the checkUpkeep() function and the data decoding in the performUpkeep() function. This mismatch prevents the liquidation process from functioning correctly.

The checkUpkeep() function encodes the performData as follows:

LiquidationKeeper.sol#L85

This encoding includes both the accountsToBeLiquidated array and the address of the LiquidationKeeper contract.

When checkUpkeep() returns upkeepNeeded == true, the Keeper will then call performUpkeep(bytes calldata peformData) with extraData as the peformData parameter.
However, in the performUpkeep() function, the decoding is performed as:

LiquidationKeeper.sol#L101

This decoding attempt only extracts the accountsToBeLiquidated array, ignoring the additional address that was encoded. Due to this mismatch, the following liquidateAccounts() call will fail on this line:

LiquidationBranch.sol#L135

Note: the liquidateAccounts.t.sol tests are actually ensuring liquidateAccounts() reverts in case of an account that does not exists. But due to this bug, it will always revert.

liquidateAccounts.tree#L8-L9

The root cause of this issue lies in the inconsistency between the data structures used for encoding and decoding. This inconsistency breaks the critical link between the checkUpkeep() and performUpkeep() functions, rendering the liquidation process inoperable.

Impact

The liquidation process will consistently fail, as the performUpkeep() function cannot correctly decode the input data.

Proof of Concept

1. The Chainlink Keeper network calls checkUpkeep().

2. checkUpkeep() identifies accounts to be liquidated and encodes them along with the keeper's address:

   bytes memory extraData = abi.encode(accountsToBeLiquidated, address(this));

   ​

   return (upkeepNeeded, extraData);

3. The Chainlink Keeper network calls performUpkeep() with the encoded data.

4. performUpkeep() attempts to decode the data, but fails due to the mismatch:

   uint128[] memory accountsToBeLiquidated = abi.decode(peformData, (uint128[]));

5. The function reverts or produces incorrect results, failing to perform the necessary liquidations.

Recommendations

To resolve this issue, ensure that the encoding and decoding operations in checkUpkeep() and performUpkeep() are consistent. There are two potential approaches:

1. Remove the keeper address from the encoded data in checkUpkeep():

1. Update the decoding in performUpkeep() to include the keeper address: