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

Incorrect State Update in claimRewards Function Leads to Denial of Service (DoS) for Users Claiming Rewards in FeeCollector Contract

oxelmiguel

Summary

This report outlines a critical vulnerability identified in the reward distribution mechanism of FeeCollector contract. The issue arises from incorrect state updates when users claim their rewards, which prevents users from claiming rewards in subsequent distributions. This vulnerability lead to a denial of service (DoS) for users attempting to claim their rightful rewards.

Vulnerability Details

The vulnerability exists in the claimRewards function, where the userRewards[user] state is incorrectly updated to totalDistributed after a user claims their rewards. This incorrect update causes the user's pending rewards to be calculated as zero in subsequent claims, even if they are eligible for additional rewards.

https://github.com/Cyfrin/2025-02-raac/blob/89ccb062e2b175374d40d824263a4c0b601bcb7f/contracts/core/collectors/FeeCollector.sol#L206

Scenario

  1. First Claim:

    • totalDistributed = 1,000,000

    • userVotingPower = 2,500

    • totalVotingPower = 50,000

    • userRewards[user] = 0

    • share = 2,500 * 1,000,000 / 50,000 = 50,000 (https://github.com/Cyfrin/2025-02-raac/blob/89ccb062e2b175374d40d824263a4c0b601bcb7f/contracts/core/collectors/FeeCollector.sol#L486)

    • pendingReward = 50,000 - 0 = 50,000 (https://github.com/Cyfrin/2025-02-raac/blob/89ccb062e2b175374d40d824263a4c0b601bcb7f/contracts/core/collectors/FeeCollector.sol#L487)

    • After the claim, userRewards[user] is incorrectly set to totalDistributed = 1,000,000.

  2. Second Claim:

    • totalDistributed = 2,000,000 (increase by 1,000,000)

    • userVotingPower = 2,500

    • totalVotingPower = 50,000

    • userRewards[user] = 1,000,000 (from last state update)

    • share = 2,500 * 2,000,000 / 50,000 = 100,000

    • pendingReward = 0 (since share < userRewards[user] ) (https://github.com/Cyfrin/2025-02-raac/blob/89ccb062e2b175374d40d824263a4c0b601bcb7f/contracts/core/collectors/FeeCollector.sol#L487)

    • The transaction revert : if (pendingReward == 0) revert InsufficientBalance();

Root Cause

The root cause of the vulnerability is the incorrect update of userRewards[user] to totalDistributed instead of the user's current share (share). This prevents users from claiming incremental rewards in future distributions.

Impact

  • Denial of Service (DoS): Users who have claimed rewards once will be unable to claim rewards in subsequent distributions, even if they are eligible.

  • Loss of Funds: Users will lose access to their rightful rewards, leading to financial losses and a breach of trust in the system.

Poc

run in FeeCollector.test.js

describe("Fee Collection DOS POC", function () {
it("will deny users to claim rewards", async function () {
// There are two veToken holders : user1 hold 1000 veToken and user2 hold 500 veToken
// We increase user2 token to 1000 to reflect the case where a user will not have the majority of the supply
// and so totalReward will always morte than what user can claim
await veRAACToken.connect(user2).lock(ethers.parseEther("500"), ONE_YEAR);
​
const amount = ethers.parseEther("100");
await feeCollector.connect(user1).collectFee(amount, 0);
await feeCollector.connect(owner).distributeCollectedFees();
let initialBalance = await raacToken.balanceOf(user1.address);
await feeCollector.connect(user1).claimRewards(user1.address);
let LastBalance = await raacToken.balanceOf(user1.address);
// user1 sucessfully claim reward the the incorrect state update is done
console.log("Claimed Rewards:", (LastBalance - initialBalance));
​
// Wait for next period
await time.increase(WEEK);
// Collect more fees
await feeCollector.connect(user1).collectFee(amount, 0);
// Distribute collected fees
await feeCollector.connect(owner).distributeCollectedFees();
// user can't claim reward because his new reward will be less than the last totalReward (user should be able to claim)
await expect(feeCollector.connect(user1).claimRewards(user1.address)).to.be.revertedWithCustomError(feeCollector, "InsufficientBalance");
​
});
});

Tools Used

The vulnerability was identified through a thorough manual review

Recommendations

To fix the vulnerability, the userRewards[user] state should be updated to the user's current share (share) instead of totalDistributed. This ensures that users can claim incremental rewards in subsequent distributions.

corrected

  1. First Claim:

    • totalDistributed = 1,000,000

    • userVotingPower = 2,500

    • totalVotingPower = 50,000

    • userRewards[user] = 0

    • share = 2,500 * 1,000,000 / 50,000 = 50,000 (https://github.com/Cyfrin/2025-02-raac/blob/89ccb062e2b175374d40d824263a4c0b601bcb7f/contracts/core/collectors/FeeCollector.sol#L486)

    • pendingReward = 50,000 - 0 = 50,000 (https://github.com/Cyfrin/2025-02-raac/blob/89ccb062e2b175374d40d824263a4c0b601bcb7f/contracts/core/collectors/FeeCollector.sol#L487)

    • After the claim, userRewards[user] is set to shares = 50,000.

  2. Second Claim:

    • totalDistributed = 2,000,000 (increase by 1,000,000)

    • userVotingPower = 2,500

    • totalVotingPower = 50,000

    • userRewards[user] = 50,000 (from last state update)

    • share = 2,500 * 2,000,000 / 50,000 = 100,000

    • pendingReward = 100,000 - 50,000 = 50,000

    • After the claim, userRewards[user] is set to shares = 100,000.

Updates

Lead Judging Commences

inallhonesty Lead Judge 3 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.