Core Contracts

Summary

The FeeCollector contract presents a vulnerability in its reward calculation mechanism. The _calculatePendingRewards function improperly utilizes the current veRAAC voting power of users instead of considering their historical voting power during the specific distribution periods.

Vulnerability Details

The vulnerability resides within the _calculatePendingRewards function, which is used by claimRewards to determine the amount of unclaimed rewards for a user:

The core issue is the use of veRAACToken.getVotingPower() and veRAACToken.getTotalVotingPower(). These functions retrieve the current voting power of the user and the current total voting power, respectively. The reward calculation should be based on the user's voting power during the period in which the rewards were accrued, not their current voting power.

Example Exploit Scenario:

Steps:

1. Distribute Fees: The protocol distributes 1000 tokens when the total veRAAC supply is 1000 (1 token per veRAAC).

2. User A Claims: User A, with 500 veRAAC, claims 500 tokens (correct).

3. User A Withdraws: User A withdraws all veRAAC, reducing their voting power to 0.

4. Second Distribution: The Protocol distributes another 1000 tokens. Total veRAAC is now 500 (2 tokens per veRAAC).

5. User A Deposits: User A quickly deposits 500 veRAAC, making the total supply 1000.

6. User A Claims Again: Current code calculates: (2000 * 500 / 1000) - 500 = 500 tokens. The correct total should be 0 (User A had 0 veRAAC during the second distribution), but they receive 500 extra tokens.

A user's veRAAC voting power can change over time due to:

• Locking more RAAC: Increases voting power.

• Extending the lock duration: Increases voting power.

• Lock expiring: Voting power decreases linearly as the lock approaches expiry.

Impact

• Critical Severity: This vulnerability allows for significant and unfair reward extraction.

• Fund Drainage: Malicious users can drain the FeeCollector's reward pool by claiming rewards they are not entitled to.

• Unfair Distribution: Honest users who maintain a consistent veRAAC balance over time will receive fewer rewards than those who exploit the vulnerability.

• Loss of Trust: The vulnerability undermines the integrity of the reward distribution system and erodes user trust in the protocol.

Tools Used

Manual Review

Recommendations

The fundamental solution is to use historical snapshots of voting power for reward calculations. The contract needs to track the total voting power and each user's voting power at the time of each reward distribution.

Here's a detailed breakdown of the recommended changes:

1. Modify FeeCollector:

   uint256 public snapshotTotalVotingPower;

   mapping(address => uint256) public snapshotUserVotingPower;

2. Modify distributeCollectedFees: Before distributing rewards, take a snapshot of the current total voting power and each user's voting power.

   function distributeCollectedFees() external override nonReentrant whenNotPaused {

   // ... (Existing checks and calculations)

   ​

   // Take a snapshot of voting power *before* distribution.

   snapshotTotalVotingPower = veRAACToken.getTotalVotingPower();

   // Assuming veRAACToken has a way to iterate through locked addresses

   for (uint256 i = 0; i < veRAACToken.getTotalLockedAddresses(); i++) {

   address user = veRAACToken.getLockedAddressAtIndex(i);

   snapshotUserVotingPower[user] = veRAACToken.balanceOf(user); // Or getVotingPower(user) if appropriate

   }

   ​

   uint256[4] memory shares = _calculateDistribution(totalFees);

   _processDistributions(totalFees, shares);

   ​

   // ...

   }

3. Modify _calculatePendingRewards: Use the snapshot values instead of the current voting power.

   function _calculatePendingRewards(address user) internal view returns (uint256) {

   // Use snapshot values instead of current values.

   uint256 userVotingPower = snapshotUserVotingPower[user];

   if (userVotingPower == 0) return 0;

   ​

   uint256 totalVotingPower = snapshotTotalVotingPower;

   if (totalVotingPower == 0) return 0;

   ​

   uint256 share = (totalDistributed * userVotingPower) / totalVotingPower;

   return share > userRewards[user] ? share - userRewards[user] : 0;

   }

4. Modify claimRewards: Correctly update userRewards[user] by adding the pendingReward, not assigning totalDistributed.

   function claimRewards(address user) external override nonReentrant whenNotPaused returns (uint256) {

   // ... (Existing checks)

   ​

   uint256 pendingReward = _calculatePendingRewards(user);

   if (pendingReward == 0) revert InsufficientBalance();

   ​

   // Correctly update userRewards.

   userRewards[user] += pendingReward; // ADD, don't assign

   ​

   // ...

   }