Zaros Part 1
Zaros
DeFiFoundry
60,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

checkUpeep encodes address(this) but performUpkeep does not use address(this)

impish

Summary

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

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

Within LiquidationKeeper.sol performUpkeep encodes the address of the Liquidation Keeper, but this passed around encoded data is never used.

Vulnerability Details

Within LiquidationKeeper.sol::performUpkeep we return the following:

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

However within LiquidationKeeper.sol::performUpkeep we only make use of the encoded accountsToBeLiquidated:

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

While abi.decode will skip over the address due to clever usage of byte headers, this is still unnecessary overhead and code bloat.

Impact

The impact is very low - only gas fees on passing around address(this) and bloated code complexity.

Tools Used

Manual Review

Recommendations

Only return and decode values that are used to avoid extraneous gas fees. Omit encoding address(this).

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Error is in decoding of `peformData`

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.