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

Incorrect userRewards update in FeeCollector contract

x1485967

Summary

We update the userRewards[user] incorrectly. This will lead that veRAAC holders will lose some rewards.

Vulnerability Details

In FeeCollector, the collected fees will be distributed periodically. One part of collected fees will be distributed to the veRAAC holders. And the veRAAC holders can claim some rewards according to their veRAAC token amount they hold via claimRewards().

function _processDistributions(uint256 totalFees, uint256[4] memory shares) internal {
...
if (shares[0] > 0) {
uint256 totalVeRAACSupply = veRAACToken.getTotalVotingPower();
if (totalVeRAACSupply > 0) {
TimeWeightedAverage.createPeriod(
distributionPeriod,
block.timestamp + 1,
7 days,
shares[0],
totalVeRAACSupply
);
totalDistributed += shares[0];
}
}

The problem happens in claimRewards. In claimRewards, we will calculate the related rewards for the user and set the claimed amount into userRewards[user] .When the user tries to claim rewards for a second time, we will deduct the userRewards[user] to get the available rewards for this user.

The problem is that we should set claimed rewards to userRewards[user], not totalDistributed. This will cause that users may fail to claim some expected rewrads.

For example:

  1. The distributor role distributes 1000 RAAC token to all veRAAC holders.

  2. Alice holds 10% veRAAC, so Alice can claim 100 RAAC. Now userRewards[Alice] is set to 1000.

  3. The distributor role distributes another 1000 RAAC token to all veRAAC holders in the second round.

  4. Alice should claim another 100 RAAC token. However, alice's claim will be reverted because the calculation share - userRewards[user] will be reverted.

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();
userRewards[user] = totalDistributed;
​
raacToken.safeTransfer(user, pendingReward);
return pendingReward;
}
​
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;
}

Impact

veRAAC holders will fail to claim some expected rewards.

Tools Used

Manual

Recommendations

- userRewards[user] = totalDistributed;
+ userRewards[user] += pendingReward;
Updates

Lead Judging Commences

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