Part 2

Summary

This issue arises from allocating arrays with a fixed, oversized length while only populating a portion of the elements in checkUpkeep. As a result, when these arrays are decoded and iterated over in performUpkeep, the function attempts to process trailing entries that remain uninitialized, forcing calls with invalid parameters (marketId=0, asset=address(0)), potentially leading to a revert. Consequently, normal fee distribution is disrupted, and the keeper’s functionality may fail entirely due to these unintended calls.

Vulnerability Details

The core problem stems from creating an array of length liveMarketIds.length * 10 in checkUpkeep but only partially populating it with valid market and asset data. Unused indices remain initialized to zero (uint128(0), address(0)). Later, in performUpkeep, the entire array is decoded and iterated through, including those zeroed entries. This causes invalid calls to convertAccumulatedFeesToWeth(0, address(0), ...), which may revert or behave unexpectedly. If the first invalid call reverts, it prevents any further fee distribution, effectively undermining the keeper’s intended functionality.

Impact

Failing to properly handle trailing zero entries can cause the entire performUpkeep function to revert on the first invalid call. This prevents all subsequent fee distributions and interrupts the keeper’s automation process. As a result, accumulated fees remain undistributed, which can halt critical protocol operations that rely on timely conversion and distribution of these assets.

Proof of Concept

During the encoding process of the performData array within checkUpkeep, the code allocates an array of size liveMarketIds.length * 10 but only partially fills it with valid market and asset data. This leaves unused slots containing zeros. When that performData is later decoded in performUpkeep, the loop attempts to process these “garbage” (trailing zeros) entries that were never intended to be used, resulting in calls like:

which may revert or exhibit unexpected behavior.

Code Analysis

• FeeConversionKeeper.sol - checkUpkeep

• FeeConversionKeeper.sol - performUpkeep

Below is a key snippet illustrating the issue of oversized arrays:

Explanation

• The array is instantiated with a length of liveMarketIds.length * 10 but only partially populated.

• Unused positions remain as zero for both uint128 and address types.

• Decoding this oversized array in performUpkeep causes iteration over all elements, including zeroed entries.

• This leads to calls like convertAccumulatedFeesToWeth(0, address(0), ...), which can revert or otherwise behave unpredictably.

Vulnerable Scenario

• There are 2 markets that require fee distribution (distributionNeeded = true).

• liveMarketIds.length could be 1, resulting in an array length of 1 * 10 = 10.

• Only 2 of those 10 entries are valid; the rest remain (0, address(0)).

• When performUpkeep is invoked, the contract decodes an array with 10 total elements and processes each one.

• By the third loop iteration (index 2), it tries convertAccumulatedFeesToWeth(0, address(0), ...), triggering a revert or undesired outcome.

This scenario disrupts performUpkeep execution and compromises fee distribution functionality.

Test and Result

This test demonstrates that once the forwarder slot is patched, calls to performUpkeep() bypass the onlyForwarder restriction and proceed to process trailing-zero array entries. The mock engine intentionally reverts with "Mock revert for trailing zeros", confirming that these oversized arrays indeed trigger unintended execution paths. The log trace shows the call to convertAccumulatedFeesToWeth() with invalid (marketId, asset), validating the vulnerability and the associated revert.

• Add the following test to test/FeeConversionKeeperTrailingZerosTest.t.sol

Confirmation

By comparing the actual number of valid items (tracked by index) to the full array length used in performData, it becomes clear that trailing zeros force the contract to process invalid entries in performUpkeep. This confirms the finding and highlights the potential for failure in the fee conversion logic.

Tools Used

Manual Code Review:
A thorough inspection of the contract’s logic and storage structures was performed to identify how the array is allocated and used, revealing the trailing zero entries and their unintended execution paths.

Recommendations

Provide only the populated portion of the array when encoding performData (e.g., by creating a new array sized exactly to the valid entries) or by passing both the full array and the actual count of meaningful elements. On decoding, rely on the real count to skip trailing zeros and avoid invalid calls such as convertAccumulatedFeesToWeth(0, address(0), ...).