Zaros Part 1

Summary

In LiquidationKeepercontract, function checkUpkeepis used to construct array of accounts to be liquidated if there is any need to liquidation. The constructed array is passed to performUpkeep. The functions encodes wrong data and thus decoding in performUpKeepwill always fail.

Vulnerability Details

In function checkUpkeep, data encoding is done as follows. This data will be passed to performUpkeepfunction if upkeepNeededis true. If it's true, that means some accounts needs to be liquidated.

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

In function performUpkeep, the data decoding is done as:

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

The above line will always revert because performDataalso contains addressin encoding. Due to this, performUpkeep will always revert and no accounts can be liquidated.

Impact

The protocol will be unable to liquidate accounts that have breached their maintenance margins, potentially leading to increased bad debt and financial loss in the system.

Tools Used

Manual review

Recommendations

There is no need to add address(this)in encoding in function checkUpkeep. It can be rewritten as follows: