Summary

Error in decoding upkeep data

Vulnerability Details

In the checkUpkeep function, the return data (performData) is encoded as follows:

However, in the performUpkeep function, the data is decoded differently:

Let's say checkUpkeep processes the following data

accountsToBeLiquidated = [1, 2, 3];
address(this) = 0x1234567890123456789012345678901234567890;

The extraData returned by checkUpkeep would be the ABI encoding of:
([1, 2, 3], 0x1234567890123456789012345678901234567890)

When performUpkeep is called with this data, it attempts to decode only the uint128[] part, ignoring the address. The address(this) included in the encoded data is completely ignored in performUpkeep

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/external/chainlink/keepers/liquidation/LiquidationKeeper.sol#L100C5-L106C6

Impact

The decoding in performUpkeep may fail or produce corrupted data because it's not decoding the full structure that was encoded. This could lead to:

• Incorrect accounts being liquidated

• The function reverting due to decoding errors

• Unpredictable behavior if partial data is incorrectly interpreted

Tools Used

Manual Review

Recommendations

In checkupkeep:

bytes memory performData = abi.encode(accountsToBeLiquidated);
return (upkeepNeeded, performData);

In PerformUpkeep:
uint128[] memory accountsToBeLiquidated = abi.decode(performData, (uint128[]));

if the address(this) is needed in performUpkeep, make sure to decode it:
(uint128[] memory accountsToBeLiquidated, address sender) = abi.decode(performData, (uint128[], address));