Summary
claimRewards() incorrectly resets user rewards after transfer to totalDistributed. As a result of this, the user won't be able to claim next batch of reward and will revert:
File: contracts/core/collectors/FeeCollector.sol
199: function claimRewards(address user) external override nonReentrant whenNotPaused returns (uint256) {
200: if (user == address(0)) revert InvalidAddress();
201:
202: uint256 pendingReward = _calculatePendingRewards(user);
203: if (pendingReward == 0) revert InsufficientBalance();
204:
205:@--->
206:@---> userRewards[user] = totalDistributed;
207:
208:
209: raacToken.safeTransfer(user, pendingReward);
210:
211: emit RewardClaimed(user, pendingReward);
212: return pendingReward;
213: }
Description
We'll walk through the vulnerable scenario using the output generated by the coded PoC provided in the later section of the report.
Output:
FeeCollector
Fee Collection and Distribution
Attempting second claim for User1...
State Changes Over Time:
┌─────────┬────────────────────────────┬───────────────────────┬──────────────────┬────────────┬───────────────────────┬────────────────────────────┬──────────────────────────┬────────────────────────┬──────────────┬──────────────────────────┐
│ (index) │ time │ action │ totalDistributed │ totalPower │ user1Power │ user1Balance │ user1Reward │ user2Power │ user2Balance │ user2Reward │
├─────────┼────────────────────────────┼───────────────────────┼──────────────────┼────────────┼───────────────────────┼────────────────────────────┼──────────────────────────┼────────────────────────┼──────────────┼──────────────────────────┤
│ 0 │ '2026-02-19T05:27:07.000Z' │ 'Initial State' │ '0.0' │ '4375.0' │ '1000.0' │ '12999.99' │ '0.0' │ '3000.0' │ '11500.0' │ '0.0' │
│ 1 │ '2026-02-19T05:27:07.000Z' │ 'Fees Collected' │ '0.0' │ '4375.0' │ '1000.0' │ '12999.99' │ '0.0' │ '3000.0' │ '11500.0' │ '0.0' │
│ 2 │ '2027-02-19T05:27:07.000Z' │ 'First Distribution' │ '999.909999' │ '4375.0' │ '750.000000000004624' │ '12999.99' │ '171.413142685715342533' │ '2250.000000000013872' │ '11500.0' │ '514.2394280571460276' │
│ 3 │ '2027-02-19T05:27:07.000Z' │ 'User1 First Claim' │ '999.909999' │ '4375.0' │ '750.000000000004624' │ '13171.403142685715342533' │ '0.0' │ '2250.000000000013872' │ '11500.0' │ '514.2394280571460276' │
│ 4 │ '2028-02-19T05:27:07.000Z' │ 'Second Distribution' │ '1999.909999' │ '4375.0' │ '500.000000000009248' │ '13171.403142685715342533' │ '0.0' │ '1500.000000000027744' │ '11500.0' │ '685.683428228584110972' │
└─────────┴────────────────────────────┴───────────────────────┴──────────────────┴────────────┴───────────────────────┴────────────────────────────┴──────────────────────────┴────────────────────────┴──────────────┴──────────────────────────┘
✔ Shows Broken FeeCollector claimRewards Functionality (250ms)
1 passing (5s)
User1's voting power = 1000 locked for 4 yrs.
User2's voting power = 3000 locked for 4 yrs.
Total voting power in the system = 4375.
After 1 year, both user's voting power reduced by a quarter to 750 and 2250 respectively. Note that totalVotingPower should've reduced too, but that's a different bug and reported separately.
Fee collected on 2027-02-19 = ~ 1000 which is the totalDistributed figure (see index 2 and 3 under column totalDistributed in the table, shown as 999.909999).
User1 reward is correctly calculated as 750/4375 * 1000 = ~ 171.4 (see index 2 under column user1Reward in the table).
User1 claims this and its RAACToken balance increases by 171.4 i.e. from 12999.99 to 13171.403 (see index 2 and 3 under column user1Balance in the table)
In code userRewards[user] = totalDistributed has been executed and hence userRewards[user] = ~ 1000.
Fee is again collected after 1 yr on 2028-02-19 = ~ 1000 which increments the totalDistributed figure to approx 2000 (see index 4 under column totalDistributed in the table, shown as 1999.909999).
The user1Reward however evaluates as zero because of the following logic inside _calculatePendingRewards():
File: contracts/core/collectors/FeeCollector.sol
479: function _calculatePendingRewards(address user) internal view returns (uint256) {
480: uint256 userVotingPower = veRAACToken.getVotingPower(user);
481: if (userVotingPower == 0) return 0;
482:
483: uint256 totalVotingPower = veRAACToken.getTotalVotingPower();
484: if (totalVotingPower == 0) return 0;
485:
486: uint256 share = (totalDistributed * userVotingPower) / totalVotingPower;
487:@---> return share > userRewards[user] ? share - userRewards[user] : 0;
488: }
share is first calculated as (2000 * 500) / 4375 = 228.57 on L486. But since 228.57 < userRewards[user] or 228.57 < 1000, zero is returned. This eventually reverts on L203 with error InsufficientBalance().
Impact
User can't claim their reward.
Proof of Concept
Add this inside the describe("Fee Collection and Distribution" section of FeeCollector.test.js and run to see it pass with the aforementioned output:
it("Shows Broken FeeCollector claimRewards Functionality", async function () {
let user3 = (await ethers.getSigners())[9];
const zeroFeeType = {
veRAACShare: 10000,
burnShare: 0,
repairShare: 0,
treasuryShare: 0
};
for (let i = 0; i < 8; i++) {
await feeCollector.connect(owner).updateFeeType(i, zeroFeeType);
}
const initialBalance = ethers.parseEther("5000");
await raacToken.mint(user1.address, initialBalance);
await raacToken.mint(user2.address, initialBalance);
await raacToken.mint(user3.address, ethers.parseEther("2000"));
await time.increase(ONE_YEAR);
await veRAACToken.connect(user1).lock(ethers.parseEther("1000"), 4 * ONE_YEAR);
await veRAACToken.connect(user2).lock(ethers.parseEther("3000"), 4 * ONE_YEAR);
const states = [];
const trackState = async (action) => {
const currentTime = await time.latest();
const date = new Date(currentTime * 1000).toISOString();
const state = {
time: date,
action,
totalDistributed: ethers.formatEther(await feeCollector.totalDistributed()),
totalPower : ethers.formatEther(await veRAACToken.getTotalVotingPower()),
user1Power : ethers.formatEther(await veRAACToken.getVotingPower(user1.address)),
user1Balance: ethers.formatEther(await raacToken.balanceOf(user1.address)),
user1Reward : ethers.formatEther(await feeCollector.getPendingRewards(user1.address)),
user2Power : ethers.formatEther(await veRAACToken.getVotingPower(user2.address)),
user2Balance: ethers.formatEther(await raacToken.balanceOf(user2.address)),
user2Reward : ethers.formatEther(await feeCollector.getPendingRewards(user2.address)),
};
states.push(state);
};
await trackState("Initial State");
const feeAmount = ethers.parseEther("1000");
await raacToken.connect(user3).approve(feeCollector.target, feeAmount);
await feeCollector.connect(user3).collectFee(feeAmount, 0);
await trackState("Fees Collected");
await time.increase(ONE_YEAR);
await feeCollector.connect(owner).distributeCollectedFees();
await trackState("First Distribution");
await feeCollector.connect(user1).claimRewards(user1.address);
await trackState("User1 First Claim");
await raacToken.connect(user3).approve(feeCollector.target, feeAmount);
await feeCollector.connect(user3).collectFee(feeAmount, 0);
await time.increase(ONE_YEAR);
await feeCollector.connect(owner).distributeCollectedFees();
await trackState("Second Distribution");
console.log("\nAttempting second claim for User1...");
await expect(feeCollector.connect(user1).claimRewards(user1.address))
.to.be.revertedWithCustomError(feeCollector, "InsufficientBalance")
.withArgs();
console.log("\nState Changes Over Time:");
console.table(states);
});
Mitigation
This will require some significant changes. Instead of resetting to totalDistributed, one needs to keep track of something like totalDistributedToUser[userAddress] and reset userRewards[user] to this value on L206. This will have to be supplemented with some index-accrual based logic because a user's voting power decays with time as it gets close to lock expiry and hence multiplying current voting power with totalDistributed can't be the share used in L487.
