Ineffective Reward Recovery Due to Incorrect Merkle Proof Verification in `recoverRewards` Function

[hats-bug-reporter[bot]]

Github username: -- Twitter username: ACai_sec Submission hash (on-chain): 0x9ac86aa605d08d80458fa9f58972eeaf0e04b5c018f35d4a4e4e4d19c854e9cb Severity: high

Description: Description:
The recoverRewards function in the smart contract is intended to allow campaign owners to recover unclaimed rewards. This function iterates through an array of ClaimRewardBundle, each intended to specify the details required to claim rewards for a campaign. A crucial step in this function involves verifying a Merkle proof to ensure that the reward claim is legitimate and conforms to the data encoded in the campaign's Merkle root.

However, a critical flaw exists in the implementation: the _processRewardClaim function is called with address(0) as the _claimOwner. This usage is problematic because the Merkle proof verification step in _processRewardClaim constructs a leaf node using the _claimOwner parameter, which should typically represent the actual owner's address. However, by passing address(0), the constructed leaf will not match any valid leaf in the Merkle tree unless explicitly designed to include such entries (which is uncommon and not secure). This will invariably cause the Merkle proof verification to fail, rendering the recoverRewards function unable to perform its intended operation, thereby locking the rewards in the contract without a means for recovery under normal operation.

Attack Scenario:
While this vulnerability does not directly lend itself to an attack that could be exploited maliciously for unauthorized gain, it represents a significant risk in terms of contract reliability and functionality. The primary risk scenario here is operational:

1. Campaign Owners Unable to Recover Funds: Campaign owners, expecting to be able to recover unclaimed rewards at the end of the campaign, find themselves unable to do so due to the constant failure of the Merkle proof verification caused by the incorrect passing of address(0) as the _claimOwner.
2. Loss of Funds: As a result, unclaimed rewards remain locked within the contract, leading to potential financial losses for campaign owners and a loss of trust in the platform's reliability.

Attachments:

1. Proof of Concept (PoC):

   The Proof of Concept (PoC) for demonstrating the issue with the recoverRewards function centers around the operational failure caused by incorrect Merkle proof verification. This is a conceptual demonstration, as the actual behavior is dependent on internal contract mechanisms which are not directly observable without interaction logs or testing in a controlled environment.

   • Scenario Setup: Consider a campaign owner who wishes to recover unclaimed rewards at the end of a campaign. The campaign is properly registered in the contract with a valid Merkle root that includes legitimate claims.
   • Action: The campaign owner initiates the reward recovery process by invoking recoverRewards, passing in a ClaimRewardBundle that includes the correct token, amount, and a valid Merkle proof corresponding to a real unclaimed reward.
   • Expected Behavior: The function should verify the Merkle proof against the campaign's stored Merkle root and allow the recovery of the specified rewards.
   • Faulty Execution: However, due to the implementation flaw, the function internally uses address(0) as the _claimOwner when constructing the leaf for Merkle proof verification. This invariably leads to a mismatch because no valid Merkle tree leaf would correspond to the zero address for a legitimate claim.
   • Outcome: The Merkle proof verification fails systematically, regardless of the correctness of the input data provided by the campaign owner. As a result, the function reverts with an "Invalid Proof" error, and the unclaimed rewards remain locked in the contract.

2. Revised Code File (Optional)

[luzzif]

The fact that unassigned (or recoverable) rewards in a campaign are assigned to the 0-address is the intended functioning of the platform. That makes it possible for the campaign to change ownership during the course of its lifetime without the need to regenerate the Merkle tree at any new ownership change. You can actually see that there are tests written to verify that the campaign owner can claim recoverable rewards using a 0-address claim here and it's working fine. Would still be glad to have a PoC if you think the issue is valid, and would also love to know why you think having 0-address claims in the Merkle tree as leaves is inherently unsecure.