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

userRewards[user] is not updated correctly in function claimRewards()

zhanmingjing

Summary

userRewards[user] is not updated correctly in function claimRewards(), which will make the user get less rewards.

Vulnerability Details

In function FeeCollector.sol#claimRewards(), pendingReward is culculated and transferred to the user. However, userRewards[user] is then updated to 'totalDistributed'. Infact, there may be quite a lot of users, totalDistributed is the Distributed amount of all users, not the Distributed value of the current user.

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;
// Transfer rewards
raacToken.safeTransfer(user, pendingReward);
emit RewardClaimed(user, pendingReward);
return pendingReward;
}

Please refer to the following steps for POC:

1, There are 3 users, A, B, and C, voting power for A is 20%, voting power for B is 30%, voting power for C is 50%,

2, totalDistributed now is 100, userA claimRewards, will get 20, then update userRewards[A] to 100.

3, later, totalDistributed is 200, userA claimRewards, in function _calculatePendingRewards(), share = 200 * 20% = 40, 40 then is compared to userRewards[A], which is 100, since 40 < 100, _calculatePendingRewards() will return 0, userA will not get any rewards.

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

users will get less rewards then expected.

Tools Used

manually review

Recommendations

In function claimRewards(), update userRewards[user] to userRewards[user] + pendingReward.

userRewards[user] = userRewards[user] + pendingReward;

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
//delete userRewards[user] = totalDistributed;
//add
userRewards[user] = userRewards[user] + pendingReward;
// Transfer rewards
raacToken.safeTransfer(user, pendingReward);
emit RewardClaimed(user, pendingReward);
return 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.