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

Users can only claim rewards once in `FeeCollector.sol` due to incorrect update of `userRewards` mapping, leading to loss of subsequent rewards.

mo_

Summary

Users can only claim rewards once in FeeCollector.sol due to incorrect update of userRewards mapping, leading to loss of subsequent rewards.

Vulnerability Details

In the claimRewards function, after calculating and transferring pendingReward to the user, the userRewards mapping is incorrectly updated. It is set to totalDistributed instead of updating it to reflect the cumulative claimed rewards. This incorrect update prevents users from claiming rewards more than once, as subsequent calls to claimRewards will calculate pendingReward as zero or a very small amount.

Problematic Code Snippet:

// contracts/core/collectors/FeeCollector.sol
​
/**
* @notice Claims accumulated rewards for a user
*/
function claimRewards(address user) external override nonReentrant whenNotPaused returns (uint256) {
// ...
uint256 pendingReward = _calculatePendingRewards(user);
if (pendingReward == 0) revert InsufficientBalance();
​
// Reset user rewards before transfer - INCORRECT LOGIC
userRewards[user] = totalDistributed; // Issue: Should be updated with claimed amount
​
// Transfer rewards
raacToken.safeTransfer(user, pendingReward);
// ...
return pendingReward;
}
​
/**
* @dev Calculates pending rewards for a user using time-weighted average
*/
function _calculatePendingRewards(address user) internal view returns (uint256) {
// ...
uint256 share = (totalDistributed * userVotingPower) / totalVotingPower;
return share > userRewards[user] ? share - userRewards[user] : 0; // Relies on userRewards for calculation
}

The line userRewards[user] = totalDistributed; incorrectly resets the user's reward record, making it impossible to claim future rewards.

Impact

  • Loss of Rewards for Users: Users are unable to claim rewards accumulated after their first claim.

  • Broken Reward System: The intended continuous reward mechanism is broken, leading to an unfair distribution of protocol fees.

Tools Used

Manual code review.

Recommendations

Correctly update the userRewards mapping in the claimRewards function. The intended logic should likely be to update userRewards[user] to the user's new cumulative reward balance after claiming.

Suggested Fix:

// contracts/core/collectors/FeeCollector.sol
...
function claimRewards(address user) external override nonReentrant whenNotPaused returns (uint256) {
// ...
uint256 pendingReward = _calculatePendingRewards(user);
if (pendingReward == 0) revert InsufficientBalance();
​
// Reset user rewards before transfer - CORRECTED LOGIC
- userRewards[user] = totalDistributed; // Incorrect: Resets to totalDistributed
+ userRewards[user] += pendingReward; // Correct: Add pendingReward to existing userRewards
​
// Transfer rewards
raacToken.safeTransfer(user, pendingReward);
// ...
return pendingReward;
}
...

By adding pendingReward to the existing userRewards[user], the user's reward balance is correctly updated, allowing them to claim rewards in subsequent periods.

Updates

Lead Judging Commences

inallhonesty Lead Judge 3 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.