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

Unfair reward distribution and claim system in FeeCollector may cause financial loss to veRAAC holders

heliosophistxiv

Summary

In FeeCollector, it is possible to retroactively collect rewards, which will then dilute or completely eliminate rewards for earlier holders.

Vulnerability Details

In FeeCollector, fees are distributed by calling a function in L180:

function distributeCollectedFees() external override nonReentrant whenNotPaused {
if (!hasRole(DISTRIBUTOR_ROLE, msg.sender)) revert UnauthorizedCaller();
uint256 totalFees = _calculateTotalFees();
if (totalFees == 0) revert InsufficientBalance();
uint256[4] memory shares = _calculateDistribution(totalFees);
_processDistributions(totalFees, shares);
delete collectedFees;
emit FeeDistributed(shares[0], shares[1], shares[2], shares[3]);
}

Which in turn calls:

function _processDistributions(uint256 totalFees, uint256[4] memory shares) internal {
uint256 contractBalance = raacToken.balanceOf(address(this));
if (contractBalance < totalFees) revert InsufficientBalance();
​
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]; // used as "index" for reward claims
} else {
shares[3] += shares[0]; // Add to treasury if no veRAAC holders
}
}
​
if (shares[1] > 0) raacToken.burn(shares[1]);
if (shares[2] > 0) raacToken.safeTransfer(repairFund, shares[2]);
if (shares[3] > 0) raacToken.safeTransfer(treasury, shares[3]);
}

The variable totalDistributed is used as an index to track user claims.

When claimRewards(address)is called, the pending rewards are calculated and the "index" value is copied to userRewards[user]to prevent double claims.

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

However, it does not take into account whether holders had veRAAC and the time of distribution, so claims are valid for past distributions.

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

Holders of veRAAC may lose their rewards if someone comes in as a new veRAAC holder and claims rewards retroactively.

Consider the following example.

Total distributed is 100, and there are 100 users where each controls 1% of the voting power. Therefore, everyone can claim 1 token.

After 99 claims, there is 1 token left in the contract to claim. However, attacker (101th user) locks veRAAC for an amount that will make his new voting power 1%. In that case, he can claim the last token, while user 100 will have nothing to claim.

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

As an alternative scenario, a user who locks his RAAC to receive veRAAC for a year can wait for his tokens to unlock, and at day 365 transfer his RAAC tokens to a new address, lock them again and claim rewards again.

Tools Used

  • Manual review

Recommendations

Consider implementing an accounting system for the rewards, so that users can't claim retroactively distributed rewards.

Updates

Lead Judging Commences

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Time-Weighted Average Logic is Not Applied to Reward Distribution in `FeeCollector`

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Time-Weighted Average Logic is Not Applied to Reward Distribution in `FeeCollector`

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!