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

The users wont be able to claim their rewards after once they've done so in the FeeCollector.sol

hard1k

Summary

The users wont be able to claim their rewards using the claimRewards function after once they've done so in the FeeCollector.sol due to the incorrect updating of the state and userRewards mapping

Vulnerability Details

Suppose a user calls the claimRewardfunction the function in the start computes pending reward of the user via _calculatePendingRewards internal function, inside this function the rewards are calculated with this formula: (totalDistributed * userVotingPower) / totalVotingPower; userVotingPower is the voting power that the user currently has, totalVotingPower is the totalSupply of the veRAACTokens. and the totalDistributed is a storage var that tracks the amount of tokens being distributed as fees or etc histroically
Lets assume: userVotingPower = 5000e18, totalVotingPower = 1000000e18 and totalDistributed = 500000e18 so reward = (500000e18 * 5000e18) / 1000000e18 = 2500e18. Now this will be the reward that will be transferred to the user in the end of the function.

However now the issue that arises here is that calculating the user's rewards the userRewards mapping is updated to totalDistributed i.e 500000e18. Now when the same user will come some time later to claim more of his rewards lets take different values(lesser than the prev example this time)for eg this time- (500000e18 * 2000e18) / 900000e18 = 1111111111111111111111 it returns tis number. Now even though this returns meaning that the user still has some rewards that he can claim, he wont be able to do so because due to this line the end reward will be returned 0. Clearly this can be seen udpating the the userRewards to totalDistributed is incorrect because even if the user will still have some rewards to claim it will always return 0 and this is because most of the time the totalDistributed variable will be a huge number as that is the all tokens that were ever distributed as fees by the contract. NOTE; See the AUDIT notes in the Code Snippets part for more details

Impact

The impact is here is kinda critical as even if the user can and will have rewards to claim he will not be able to do so.

Tools Used

Manual Review

Recommendations

Instead of updating the userRewards mapping to totalDistributed update it to the pendingReward that was computed as such like this: userRewards[user] = pendingReward;`

Code Snippets

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; //AUDIT Firstly this is being updated to totalDistributed, and
//what if the the user increases his lock in the veRAAC, now after that even though his voting power will be increased but then too
//he might not be able to claim his rewards after once he has claimed them
}```
``` function claimRewards(address user) external override nonReentrant whenNotPaused returns (uint256) {
if (user == address(0)) revert InvalidAddress();
uint256 pendingReward = _calculatePendingRewards(user);//<-AUDIT The user wont be able to claim rewards more than once because of the
//calculation in the function and also because of the state update below
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;
}
​
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!