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

Wrong calculation of users pending rewards in FeeCollector leads to periodic DoS of `claimRewards` function and loss of rewards for veRAAC holders..

tadev

Summary

When a veRAAC holder claims rewards with claimRewards function in FeeCollector contract, pending rewards are calculated as follows:

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;
}

The problem is that this function assumes userRewards[user]will return the rewards already claimed by the user, but it is not the case. Indeed, claimRewards function executes the following line:

userRewards[user] = totalDistributed;

This means each time a user claims rewards, userRewards[user]is set to the total veRAAC distributed by the contract (already claimed or not, by all users).

This will lead to DoS of the claimRewards function and wrong calculation of rewards amount leading to loss of rewards fo veRAAC holders.

Vulnerability Details

Consequences of this vulnerability are:

  • First claim will happen normally, with share > userRewards[user]and share being claimed (not the correct amount, but this is another issue).

  • For the second claim, _calculatePendingRewards will return 0, because share won't be greater than userRewards[user]which is set to totalDistributed. User will have to wait for share to be greater than userRewards[user] (totalDistributed at the moment of last claim). If a user has 1% of veRAAC token supply, he will have to wait for totalDistributed to be 100 more bigger than when he first claimed. Once share is greater than userRewards[user](totalDistributed), the reward for the user is calculated with share - userRewards[user]which is incorrect because userRewards[user] represents total distributed rewards instead of total rewards claimed by the user, leading to a way lower amount of rewards than expected.

This means :

  • The less veRAAC tokens a user holds, the more time the DoS will last after first claim, because more fees need to be distributed to make _calculatePendingRewards not return 0

  • The more totalDistributed grows, the more the DoS increases between 2 claims.

But the biggest problem is that after first claim, claimable fees are way lower than they should.

Impact

The impact of this issue is high, given that it leads to loss of rewards for user + denial of service of claimRewards function after first claim

Tools Used

Manual review.

Recommendations

Make sure that userRewards[user] indeed tracks rewards claimed per user.

userRewards[user] += pendingReward;
Updates

Lead Judging Commences

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