Core Contracts

Vulnerability Details

At FeeController::_calculatePendingRewards(), the amount of accrued rewards per user is wrongly calculated.

This is because a % of totalDistributed is calculated as the reward, but totalDistributed is a cummulatively increasing value, representing all rewards distributed over time. This means that for every new batch of rewards a user that claims will get a % of the new batch but also a % of all previous batches.

Impact

Users claim more rewards than they should after the first distribution of rewards. This means not only incorrect accounting but that eventually someone will get none rewards then they should have gotten.

Proof Of Concept

• See the computation of the % for each user here.

• See the cummulative tracking nature of totalDistributed here.

Numerical examples:

• First distribution:

  • Distribute 100 tokens.

  • User share is 10% = 10 tokens. ✅

  • User claims 10 tokens. Then userClaim[user] = totalDistributed = 100. See here.

• Second distribution:

  • Distribute 100 tokens.

  • User share is 10% = 10 tokens.

  • User tries to claim 10 tokens. But the return check does:

    • share = (200 * 10) / 100 = 20. ❎

    • User should get 10% of the new 100, 10 tokens, not 20 due to 10% of 200.

Recommendations

totalDistributed must not be used on _calculatePendingRewards(). Instead, a value representing the last batch of rewards distributed should be used.

As of now users will always claim their % of the total, instead of the total - alreadyClaimed or of the new batches. The code needs new logic and variables to track properly amounts claimed historically and from new batches.