Core Contracts

Summary

The claimRewards function in the FeeCollector contract incorrectly updates the userRewards mapping using totalDistributed instead of the calculated pendingReward. This error leads to inaccurate tracking of user rewards, potentially causing users to lose their rightful rewards or claim incorrect amounts.

Vulnerability Details

The issue is located in the claimRewards function of the FeeCollector contract:

The function calculates the pendingReward for the user using _calculatePendingRewards(user). However, after calculating the reward, it incorrectly updates the userRewards mapping with totalDistributed instead of incrementing it by pendingReward. This results in the user being unable to claim further rewards because the userRewards[user] value is overwritten with totalDistributed, which resets their reward tracking.

Explanation of the Issue

The _calculatePendingRewards function calculates the user's share of rewards based on their voting power relative to the total voting power. It then checks if the calculated share is greater than the user's previously recorded rewards (userRewards[user]). If so, it returns the difference (share - userRewards[user]) as the pendingReward. Otherwise, it returns 0.

Why This Breaks Reward Claims

When the claimRewards function is called, it calculates the pendingReward correctly. However, instead of updating userRewards[user] to reflect the newly claimed rewards, it overwrites it with totalDistributed:

This means:

1. Overwriting Reward History: The user's reward history is reset to totalDistributed, which is the cumulative amount distributed to all users up to that point. This erases the user's individual reward tracking.

2. Future Reward Claims Blocked: In subsequent calls to _calculatePendingRewards, the share will always be compared to totalDistributed (which is now stored in userRewards[user]). Since share is calculated as (totalDistributed * userVotingPower) / totalVotingPower, it will always be less than or equal to totalDistributed. As a result, the condition share > userRewards[user] will never be true, and the function will always return 0 for future claims.

3. Permanent Loss of Rewards: The user will permanently lose the ability to claim any further rewards, even if they continue to hold voting power and new fees are distributed.

Example Scenario

1. Initial State:

   • totalDistributed = 1000e18

   • userRewards[user] = 500e18

   • userVotingPower = 100

   • totalVotingPower = 1000

   • share = (1000e18 * 100) / 1000 = 100e18

   • pendingReward = 100e18 - 500e18 = 0 (since 100e18 < 500e18)

2. After Incorrect Update:

   • userRewards[user] = totalDistributed = 1000e18

   • In the next distribution cycle, even if totalDistributed increases to 2000e18, the share calculation will be:

     • share = (2000e18 * 100) / 1000 = 200e18

     • pendingReward = 200e18 - 1000e18 = 0 (since 200e18 < 1000e18)

Thus, the user can never claim rewards again.

Impact

Users lose their rewards.

Tools Used

Manual Review

Recommendations

To fix this issue, the userRewards[user] should be updated by adding the pendingReward instead of overwriting it with totalDistributed. Here is the corrected code:

This ensures that the user's reward history is accurately tracked, and they can continue to claim rewards in future distribution cycles.