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

Incorrect Reward Calculation Prevents Subsequent Claims

0xslowbug

Summary

The vulnerability identified is that users are unable to claim subsequent rewards after their initial claim due to incorrect tracking of userRewards[user]. When a user claims rewards, userRewards[user] is updated to the totalDistributed value instead of their individual share. This causes subsequent pending rewards calculations to yield zero, even when users should be eligible for additional rewards.

Affected Code: FeeCollector::ClaimReward

Vulnerability Details

The calculation of pending rewards is as follows:

uint256 share = (totalDistributed * userVotingPower) / totalVotingPower;
return share > userRewards[user] ? share - userRewards[user] : 0;

When userRewards[user] is set to the totalDistributed instead of the user's individual reward share, subsequent claims fail. For instance:

  • Initial scenario:

    • User voting power: 5

    • Total voting power: 100

    • Distributed rewards: 50

The user's share: (50 * 5) / 100 = 2.5.
After claiming this reward, userRewards[user] is set to 50 instead of 2.5.

  • Subsequent scenario:

    • User voting power: 7

    • Total voting power: 100

    • New distributed rewards: 50 (totalDistributed remains 50)

The user's share: (50 * 7) / 100 = 3.5.
The pending rewards check compares 3.5 against userRewards[user] (which is 50), resulting in zero because 3.5 < 50.

Thus, despite being eligible for rewards due to increased voting power, the user cannot claim them.

Proof of Concept (PoC)

Paste the following code into the FeeCollector.test.js file in the Fee Collection and Distribution section

it.only("should fail to distribute new rewards after initial distribution due to userRewards update", async function () {
// Get signers for owner and test users
const [owner, user1, user2, user3, user4] = await ethers.getSigners();
​
// Define the lock amount for veRAAC tokens
const lockAmount = ethers.parseEther("1000");
​
// Mint RAAC tokens and approve them for locking for each user
for (const user of [user1, user2, user3, user4]) {
await raacToken.mint(user.address, lockAmount);
await raacToken.connect(user).approve(veRAACToken.getAddress(), lockAmount);
}
​
// Lock tokens for each user for one year
for (const user of [user1, user2, user3, user4]) {
await veRAACToken.connect(user).lock(lockAmount, ONE_YEAR);
}
​
// Increase time by one week to simulate passage of time for rewards accrual
await time.increase(WEEK);
​
// Collect and distribute initial fees
const initialFeeAmount = ethers.parseEther("100");
await feeCollector.connect(user1).collectFee(initialFeeAmount, 0);
await feeCollector.connect(owner).distributeCollectedFees();
​
// Increase time by another week to allow rewards to accumulate
await time.increase(WEEK);
​
// Record initial balances before claiming rewards
const initialBalances = {};
for (const user of [user1, user2, user3, user4]) {
initialBalances[user.address] = await raacToken.balanceOf(user.address);
}
​
// Users claim their rewards
for (const user of [user1, user2, user3, user4]) {
await feeCollector.connect(user).claimRewards(user.address);
}
​
// Verify that each user's balance increased after claiming rewards
for (const user of [user1, user2, user3, user4]) {
const afterClaimBalance = await raacToken.balanceOf(user.address);
expect(afterClaimBalance).to.be.gt(initialBalances[user.address]);
}
​
// Simulate a second round of fee distribution
const newFeeAmount = ethers.parseEther("200");
await feeCollector.connect(user1).collectFee(newFeeAmount, 0);
await feeCollector.connect(owner).distributeCollectedFees();
await time.increase(WEEK);
​
// Expect subsequent reward claims to revert with "InsufficientBalance"
for (const user of [user1, user2, user3, user4]) {
await expect(
feeCollector.connect(user).claimRewards(user.address)
).to.be.revertedWithCustomError(feeCollector, "InsufficientBalance");
}
​
// Validate that user rewards and total distributed amounts remain unchanged
for (const user of [user1, user2, user3, user4]) {
const userRewardsValue = await feeCollector.getPendingRewards(user.address);
const totalDistributedValue = await feeCollector.totalDistributed();
​
// Ensure no new rewards were assigned
expect(userRewardsValue).to.equal(0);
​
// Confirm balances remain the same after the failed claim
const finalBalance = await raacToken.balanceOf(user.address);
const balanceAfterFirstClaim = initialBalances[user.address].add(
afterClaimBalance.sub(initialBalances[user.address])
);
expect(finalBalance).to.equal(balanceAfterFirstClaim);
}
});

This PoC demonstrates that after the first claim, subsequent reward claims fail even when the user should be eligible for additional rewards.

Impact

Users who have locked tokens and are eligible for additional rewards after the initial claim cannot access their rewards. This directly impacts the fairness and functionality of the reward distribution system.

Tools Used

  • Manual code review

Recommendations

  1. Track individual user shares correctly instead of updating userRewards[user] to the totalDistributed.

  2. Introduce a time-lock mechanism to prevent multiple claims within a short window.

  3. Consider maintaining a lastClaimedReward variable per user to ensure correctness.

Updates

Lead Judging Commences

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Appeal created

0xslowbug Submitter
4 months ago
inallhonesty Lead Judge
4 months ago
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.