sherlock-audit / 2024-10-gamma-rewarder-judging

7 stars 3 forks source link

MaslarovK - Broken Rewards Distribution Logic in `GammaRewarder.sol` #217

Open sherlock-admin2 opened 2 weeks ago

sherlock-admin2 commented 2 weeks ago

MaslarovK

Medium

Broken Rewards Distribution Logic in GammaRewarder.sol

Summary

The GammaRewarder.sol contract contains a critical flaw in its rewards distribution logic. Specifically, when a distribution is created, the calculated distributionAmountPerEpochmay exceed the total available rewards (realAmountToDistribute). This leads to an incorrect initial payout which depletes the funds prematurely, thereby blocking subsequent valid claims due to the restrictiveclaim.amountlogic inhandleProofResult()`. As a result, the protocol invariant, which mandates that total distributed rewards must match the initial deposit (minus protocol fees), is violated, rendering the contract's distribution mechanism unreliable.

Root Cause

The primary cause is a logical misalignment between realAmountToDistribute and distributionAmountPerEpoch. The contract sets distributionAmountPerEpoch based on the intended epoch length and total amount available. However, if this calculated per-epoch amount is higher than the available rewards, the initial payout will consume the entire balance. This issue is exacerbated by the restrictive check require(claim.amount == 0, "Already claimed reward."), which prevents any further valid claims.

Key Code References:

uint256 amountPerEpoch = realAmountToDistribute / ((_endBlockNum - _startBlockNum) / blocksPerEpoch);
claimed[userAddress][rewardTokenAddress][distributionId] = claim;
require(claim.amount == 0, "Already claimed reward.");

Internal pre-conditions

  1. realAmountToDistribute is derived by deducting the protocol fee from the user-provided amount.
  2. distributionAmountPerEpoch is calculated as a quotient of realAmountToDistribute and the number of epochs derived from the distribution duration and blocksPerEpoch.
  3. The handleProofResult() function enforces a check (claim.amount == 0) which limits each user to a single claim.

External pre-conditions

  1. The protocolFee and blocksPerEpoch values remain constant during distribution creation and handling but can lead to improper distributions due to these initial calculations.
  2. Users depend on multiple claims over the distribution’s lifetime, but the restrictive claim logic blocks subsequent valid claims.

Attack Path

This issue impacts distribution fulfillment and user claims, especially when calculations result in an amountPerEpoch that overshoots realAmountToDistribute. Here's a detailed breakdown using example values:

  1. Scenario:

    • Let’s assume realAmountToDistribute = 3e+21 tokens.
    • The calculated distributionAmountPerEpoch is mistakenly set to 4.85e+21 due to an incorrectly large per-epoch amount.
  2. Example Workflow:

    • The user creates a distribution with the following conditions:
      • distributionAmountPerEpoch = 4.85e+21
      • realAmountToDistribute = 3e+21
    • On the first claim, handleProofResult() will process 4.85e+21 tokens.
  3. Failure in Claim Logic:

    • After the initial claim, the contract’s balance for this distribution is zero (since the amount per epoch was erroneously larger than realAmountToDistribute).
    • For any subsequent claim, handleProofResult() checks claim.amount == 0, but as claim.amount is already set to 4.85e+21, the function reverts with Already claimed reward, preventing further valid claims.

Impact

This bug severely compromises the rewards distribution mechanism:

  1. Invariant Violation:

    • The protocol's invariant — ensuring total distributed rewards match the initial deposit minus fees — is broken as it blocks incremental claims due to overestimated payouts.
  2. User Funds and Trust:

    • Users are misled by the initial distribution amount calculation, as they expect periodic rewards that are ultimately withheld.
    • Users’ trust in the distribution process is eroded, as the mechanism fails to allocate rewards correctly, leading to unmet reward expectations.

PoC

No response

Mitigation

  1. Correct Calculation of distributionAmountPerEpoch:

    • Ensure distributionAmountPerEpoch is properly derived from realAmountToDistribute by adjusting for epoch-based allocations, avoiding any individual payout exceeding the remaining distributable funds.
    • Implement a more granular tracking mechanism within handleProofResult() to allow cumulative, incremental claims over time, removing the single-claim limitation.
  2. Revise Claim Logic:

    • Update handleProofResult() to maintain a cumulative record of claimed amounts across epochs, removing the claim.amount == 0 check in favor of a comparison against the remaining balance for each epoch, facilitating multiple claims over the distribution duration.