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

[H-03] _calculatePendingRewards() in FeeCollector results in user being unable to withdraw funds

attentioniayn

Summary

The way the accounting of claimed rewards is done in claimRewards() results in the user being unable to withdraw all of their claimable rewards after their first claim.

Vulnerability Details

This is due to the assignment userRewards[user] = totalDistributed in claimRewards() and the condition in the return statement of _calculatePendingRewards():

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

After the first claim, userRewards[user] will be set to totalDistributed, therefore for the condition to be true again, either the relative voting power of the user must increase or the share of the user must surpass the previously assigned totalDistributed, this is not guaranteed to happen.

The condition that needs to be satisfied becomes (totalDistributed * userVotingPower) / totalVotingPower > oldTotalDistributed, which is equivalent to userVotingPower / totalVotingPower > oldTotalDistributed / totalDistributed, which implies that the user will be able to do a partial claim (the diff between share and oldTotalDistributed) only after it's share of the votes is greater than the ratio between the old and new totalDistributed, this may not happen for quite a while.

I.e. if the user has 20% of the votes, then they will only be able to claim again after the reward is 5 times what they already claimed.

Location

Location

Impact

Part of the rewards claimable by the user will be stuck, at least until the totalDistributed value or the votes share of the user increase.

Tools Used

Manual review.

Recommendations

Instead of computing uint256 share = (totalDistributed * userVotingPower) / totalVotingPower; in _calculatePendingRewards() on the total amount distributed, compute it only on the difference between the current totalDistributed and the totalDistributed value stored in `userRewards[user]':

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

Proof of Code

This snippet shows how even after doing a second feeCollect after user2 claims their rewards, the FeeCollector won't give the user any additional rewards until the inequality in line 487 is satisfied, and even then only a part of the rewards will be given.

import { expect } from "chai";
import hre from "hardhat";
const { ethers } = hre;
import { time, mine } from "@nomicfoundation/hardhat-network-helpers";
import { deployContracts } from './utils/deployContracts.js';
​
describe('Exploit Tests', function () {
// Set higher timeout for deployments
this.timeout(300000); // 5 minutes
​
let contracts;
let owner, user1, user2, user3, treasury, repairFund;
const INITIAL_MINT_AMOUNT = ethers.parseEther('1000');
const HOUSE_TOKEN_ID = '1021000';
const HOUSE_PRICE = ethers.parseEther('100');
const ONE_YEAR = 365 * 24 * 3600;
const FOUR_YEARS = 4 * ONE_YEAR;
const BASIS_POINTS = 10000;
​
before(async function () {
[owner, user1, user2, user3, treasury, repairFund] = await ethers.getSigners();
contracts = await deployContracts(owner, user1, user2, user3);
const displayContracts = Object.fromEntries(Object.entries(contracts).map(([key, value]) => [key, value.target]));
console.log(displayContracts);
​
// Set house price for testing
await contracts.housePrices.setHousePrice(HOUSE_TOKEN_ID, HOUSE_PRICE);
​
// Mint initial tokens to users
for (const user of [user1, user2, user3]) {
await contracts.crvUSD.mint(user.address, INITIAL_MINT_AMOUNT);
}
});
​
describe.only('Bugs:', function () {
​
it('[H-03] _calculatePendingRewards() in FeeCollector results in user being unable to withdraw funds', async function () {
const TRANSFER_AMOUNT = ethers.parseEther('1');
​
// Get some veRAACToken in order to receive rewards, two users to avoid edge case
await contracts.raacToken.connect(user2).approve(contracts.veRAACToken.target, TRANSFER_AMOUNT);
await contracts.veRAACToken.connect(user2).lock(TRANSFER_AMOUNT, 3600 * 24 * 365);
await contracts.raacToken.connect(user3).approve(contracts.veRAACToken.target, TRANSFER_AMOUNT);
await contracts.veRAACToken.connect(user3).lock(TRANSFER_AMOUNT, 3600 * 24 * 365);
​
// Collect fee
await contracts.raacToken.connect(user2).approve(contracts.feeCollector.target, TRANSFER_AMOUNT);
await contracts.feeCollector.connect(user2).collectFee(TRANSFER_AMOUNT, 0);
console.log("Amount of fees in feeCollector: " + await contracts.raacToken.balanceOf(contracts.feeCollector.target)); // 1000000000000000000
​
// Distribute fees, including veRaactoken holders
console.log("Amount of fees for user2: " + await contracts.feeCollector.getPendingRewards(user2.address)); // 0
console.log("Amount of fees for user3: " + await contracts.feeCollector.getPendingRewards(user3.address)); // 0
await contracts.feeCollector.connect(owner).distributeCollectedFees();
await contracts.feeCollector.connect(user2).claimRewards(user2.address);
console.log("Amount of totalDistributed: " + await contracts.feeCollector.totalDistributed()); // 800000000000000000
console.log("Amount of fees for user2 (after claim): " + await contracts.raacToken.balanceOf(user2.address)); // 998799999949264332832
console.log("Amount of fees for user3 (after claim): " + await contracts.feeCollector.getPendingRewards(user3.address)); // 399999974632166416
console.log("Amount of fees in feeCollector (after claim): " + await contracts.raacToken.balanceOf(contracts.feeCollector.target)); // 50735667168
​
// Collect more fee for 2nd claim
await contracts.raacToken.connect(user2).approve(contracts.feeCollector.target, TRANSFER_AMOUNT);
await contracts.feeCollector.connect(user2).collectFee(TRANSFER_AMOUNT, 0);
console.log("Amount of fees in feeCollector: " + await contracts.raacToken.balanceOf(contracts.feeCollector.target)); // 1000000000000000000
​
// Wait a while (at least 7 days)
await mine(1000000);
​
// Distribute 2nd collect
await contracts.feeCollector.connect(owner).distributeCollectedFees();
console.log("Amount of fees for user2: " + await contracts.feeCollector.getPendingRewards(user2.address)); // 0
console.log("Amount of fees for user3: " + await contracts.feeCollector.getPendingRewards(user3.address)); // 774632141048166416
​
// Collect more fee for 2nd claim
await contracts.raacToken.connect(user2).approve(contracts.feeCollector.target, TRANSFER_AMOUNT);
await contracts.feeCollector.connect(user2).collectFee(TRANSFER_AMOUNT, 0);
console.log("Amount of fees in feeCollector: " + await contracts.raacToken.balanceOf(contracts.feeCollector.target)); // 1000000000000000000
​
// Wait a while (at least 7 days)
await mine(1000000);
​
// Distribute 2nd collect
await contracts.feeCollector.connect(owner).distributeCollectedFees();
console.log("Amount of fees for user2: " + await contracts.feeCollector.getPendingRewards(user2.address)); // 323896499248000000
console.log("Amount of fees for user3: " + await contracts.feeCollector.getPendingRewards(user3.address)); // 1123896537299750376
});
});
});
Updates

Lead Judging Commences

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

Give us feedback!