Core Contracts
Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Incorrect Tracking of User Rewards in FeeCollector Leads to Loss of Unclaimed Rewards

ok567

Finding Description and Impact

The function _calculatePendingRewards() miscalculates a user's pending rewards due to incorrect tracking of userRewards[user].

  • userRewards[user] is only updated whenclaimRewards() is called and it is set to totalDistributed.

    // * - totalDistributed Total tokens distributed historically

  • This means userRewards[user] is either 0 (if the user has never claimed rewards) or totalDistributed (if they have claimed at least once).

  • However, _calculatePendingRewards() assumes userRewards[user] tracks the user's actual "Accumulated rewards", which is incorrect after a user's first claim, as it is just reset to totalDistributed.

Impact

  • Users are not able to claim their entitled rewards after their first claim.

  • If a user's share of the rewards is slightly above their current accumlated rewards, they may be eligible for additional rewards, but the function will return 0 instead of the correct pending amount, as the new calculated share can never be bigger than totalDistributed.

  • This could lead to permanent loss of rewards for affected users.

Proof of Concept

function _calculatePendingRewards(address user) internal view returns (uint256) {
uint256 userVotingPower = veRAACToken.getVotingPower(user);
if (userVotingPower == 0) return 0;
​
uint256 totalVotingPower = veRAACToken.getTotalVotingPower();
if (totalVotingPower == 0) return 0;
uint256 share = (totalDistributed * userVotingPower) / totalVotingPower;
return share > userRewards[user] ? share - userRewards[user] : 0;
// after the first claim the new calculated share can never be bigger than userRewards[user]
// as it is set to totalDistributed after the first claim
}
​

Issue in _calculatePendingRewards()

The comparison share > userRewards[user] assumes userRewards[user] represents accumulated rewards received, but in reality:

  • userRewards[user] is only updated in claimRewards() as totalDistributed after a user's first claim.

  • After a user's first claim this will cause all future pending rewards to be lost.

Code Reference in claimRewards()

function claimRewards(address user) external override nonReentrant whenNotPaused returns (uint256) {
if (user == address(0)) revert InvalidAddress();
uint256 pendingReward = _calculatePendingRewards(user);
if (pendingReward == 0) revert InsufficientBalance();
// Reset user rewards before transfer
userRewards[user] = totalDistributed; // Incorrectly overwrites userRewards[user]
​
// Transfer rewards
raacToken.safeTransfer(user, pendingReward);
emit RewardClaimed(user, pendingReward);
return pendingReward;

This overwrites userRewards[user] instead of incrementally tracking rewards.

Expected Behavior

  • userRewards[user] should properly track the total claimed rewards, not just reset to totalDistributed.

Recommended Mitigation Steps

To ensure accurate tracking of claimed rewards and prevent loss of pending rewards:

Fix the reward update logic in claimRewards()
Instead of setting userRewards[user] = totalDistributed, increment it correctly:

userRewards[user] += pendingReward;

This ensures userRewards[user] correctly reflects cumulative rewards received.

By implementing these fixes, the contract will ensure that users can claim all their rightful rewards without unintended losses.

Updates

Lead Judging Commences

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

FeeCollector::claimRewards sets `userRewards[user]` to `totalDistributed` seriously grieving users from rewards

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!