Core Contracts

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

Incorrect distribution logic in `FeeCollector::_processDistributions` leads to stuck funds in `Treasury`

Summary

The purpose of FeeCollector::_processDistributions is to distribute the collected fees amongst the veRAAC holders, the repairFund and the Treasury. However, it when transferring the appropriate shares to the Treasury they get transferred through the safeTransfer function instead of Treasury::deposit. This leads to funds getting stuck as the funds transferred to the Treasury contract do not get accounted.

Vulnerability Details

FeeCollector::_processDistributions:

function _processDistributions(uint256 totalFees, uint256[4] memory shares) internal {
uint256 contractBalance = raacToken.balanceOf(address(this));
if (contractBalance < totalFees) revert InsufficientBalance();
if (shares[0] > 0) {
uint256 totalVeRAACSupply = veRAACToken.getTotalVotingPower();
if (totalVeRAACSupply > 0) {
TimeWeightedAverage.createPeriod(
distributionPeriod,
block.timestamp + 1,
7 days,
shares[0],
totalVeRAACSupply
);
totalDistributed += shares[0];
} else {
shares[3] += shares[0]; // Add to treasury if no veRAAC holders
}
}
if (shares[1] > 0) raacToken.burn(shares[1]);
if (shares[2] > 0) raacToken.safeTransfer(repairFund, shares[2]);
// @audit-issue Funds will get stuck
if (shares[3] > 0) raacToken.safeTransfer(treasury, shares[3]);
}

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

_processDistributions transfers the appropriate shares to the Treasury contract.

However, since they do NOT get transferred by Treasury::deposit they do not get accounted:

function deposit(address token, uint256 amount) external override nonReentrant {
if (token == address(0)) revert InvalidAddress();
if (amount == 0) revert InvalidAmount();
IERC20(token).transferFrom(msg.sender, address(this), amount);
@> _balances[token] += amount;
@> _totalValue += amount;
emit Deposited(token, amount);
}

https://github.com/Cyfrin/2025-02-raac/blob/89ccb062e2b175374d40d824263a4c0b601bcb7f/contracts/core/collectors/Treasury.sol#L46-L55

The _balances and _totalValue do not get updated and when the manager of the Treasury contract decides to withdraw the fees from the treasury the transaction will fail because Treasury::withdraw will ensure that there is enough tokens in balances[RAACToken] to withdraw:

function withdraw(
address token,
uint256 amount,
address recipient
) external override nonReentrant onlyRole(MANAGER_ROLE) {
if (token == address(0)) revert InvalidAddress();
if (recipient == address(0)) revert InvalidRecipient();
@> if (_balances[token] < amount) revert InsufficientBalance();
@> _balances[token] -= amount;
@> _totalValue -= amount;
IERC20(token).transfer(recipient, amount);
emit Withdrawn(token, amount, recipient);
}

https://github.com/Cyfrin/2025-02-raac/blob/89ccb062e2b175374d40d824263a4c0b601bcb7f/contracts/core/collectors/Treasury.sol#L64-L78

The same issue appears in FeeCollector::emergencyWithdraw.

PoC

import { expect } from "chai";
import hre from "hardhat";
const { ethers } = hre;
import { time } from "@nomicfoundation/hardhat-network-helpers";
describe("FeeCollector", function () {
let raacToken, feeCollector, veRAACToken;
let owner, user1, user2, treasury, repairFund, emergencyAdmin;
let defaultFeeType;
const BASIS_POINTS = 10000;
const WEEK = 7 * 24 * 3600;
const ONE_YEAR = 365 * 24 * 3600;
const INITIAL_MINT = ethers.parseEther("10000");
const SWAP_TAX_RATE = 100; // 1%
const BURN_TAX_RATE = 50; // 0.5%
beforeEach(async function () {
[owner, user1, user2, repairFund, emergencyAdmin] = await ethers.getSigners();
// Deploy RAACToken
const RAACToken = await ethers.getContractFactory("RAACToken");
raacToken = await RAACToken.deploy(owner.address, SWAP_TAX_RATE, BURN_TAX_RATE);
await raacToken.waitForDeployment();
// Deploy veRAACToken
const VeRAACToken = await ethers.getContractFactory("veRAACToken");
veRAACToken = await VeRAACToken.deploy(await raacToken.getAddress());
await veRAACToken.waitForDeployment();
// Deploy Treasury
const Treasury = await ethers.getContractFactory("Treasury");
treasury = await Treasury.deploy(owner.address);
// Deploy FeeCollector
const FeeCollector = await ethers.getContractFactory("FeeCollector");
feeCollector = await FeeCollector.deploy(
await raacToken.getAddress(),
await veRAACToken.getAddress(),
treasury.getAddress(),
repairFund.address,
owner.address
);
await feeCollector.waitForDeployment();
// Setup initial configuration
await raacToken.setFeeCollector(await feeCollector.getAddress());
await raacToken.manageWhitelist(await feeCollector.getAddress(), true);
await raacToken.manageWhitelist(await veRAACToken.getAddress(), true);
await raacToken.setMinter(owner.address);
await veRAACToken.setMinter(owner.address);
// Setup roles
await feeCollector.grantRole(await feeCollector.FEE_MANAGER_ROLE(), owner.address);
await feeCollector.grantRole(await feeCollector.EMERGENCY_ROLE(), emergencyAdmin.address);
await feeCollector.grantRole(await feeCollector.DISTRIBUTOR_ROLE(), owner.address);
// Mint initial tokens and approve
await raacToken.mint(user1.address, INITIAL_MINT);
await raacToken.mint(user2.address, INITIAL_MINT);
await raacToken.connect(user1).approve(await feeCollector.getAddress(), ethers.MaxUint256);
await raacToken.connect(user2).approve(await feeCollector.getAddress(), ethers.MaxUint256);
await raacToken.connect(user1).approve(await veRAACToken.getAddress(), ethers.MaxUint256);
await raacToken.connect(user2).approve(await veRAACToken.getAddress(), ethers.MaxUint256);
// Setup initial fee types
defaultFeeType = {
veRAACShare: 5000, // 50%
burnShare: 1000, // 10%
repairShare: 1000, // 10%
treasuryShare: 3000 // 30%
};
for (let i = 0; i < 8; i++) {
await feeCollector.connect(owner).updateFeeType(i, defaultFeeType);
}
// Calculate gross amounts needed including transfer tax
const taxRate = SWAP_TAX_RATE + BURN_TAX_RATE; // 150 basis points (1.5%)
const grossMultiplier = BigInt(BASIS_POINTS * BASIS_POINTS) / BigInt(BASIS_POINTS * BASIS_POINTS - taxRate * BASIS_POINTS);
const protocolFeeGross = ethers.parseEther("50") * grossMultiplier / BigInt(10000);
const lendingFeeGross = ethers.parseEther("30") * grossMultiplier / BigInt(10000);
const swapTaxGross = ethers.parseEther("20") * grossMultiplier / BigInt(10000);
// Collect fees
await feeCollector.connect(user1).collectFee(protocolFeeGross, 0);
await feeCollector.connect(user1).collectFee(lendingFeeGross, 1);
await feeCollector.connect(user1).collectFee(swapTaxGross, 6);
// Create veRAACToken locks
await veRAACToken.connect(user1).lock(ethers.parseEther("1000"), ONE_YEAR);
await time.increase(WEEK);
await veRAACToken.connect(user2).lock(ethers.parseEther("500"), ONE_YEAR);
});
it("incorrectly tranfers funds to the Treasury contract", async () => {
// collect fees
for (let feeType = 0; feeType < 8; feeType++) {
await feeCollector.connect(user1).collectFee(ethers.parseEther("1000"), feeType);
}
// distribute fees
await expect(feeCollector.connect(owner).distributeCollectedFees()).to.emit(feeCollector, "FeeDistributed");
const balance = await raacToken.balanceOf(treasury.getAddress());
await expect(treasury.connect(owner).withdraw(raacToken.getAddress(), balance, owner.address))
.to.be.revertedWithCustomError(treasury, "InsufficientBalance");
});
});

Impact

The issue leads to funds getting stuck.

Tools Used

VSCode, Manual Research

Recommendations

Modify the FeeCollector::_processDistributions:

function _processDistributions(uint256 totalFees, uint256[4] memory shares) internal {
uint256 contractBalance = raacToken.balanceOf(address(this));
if (contractBalance < totalFees) revert InsufficientBalance();
...SKIP...
if (shares[1] > 0) raacToken.burn(shares[1]);
if (shares[2] > 0) raacToken.safeTransfer(repairFund, shares[2]);
- if (shares[3] > 0) raacToken.safeTransfer(treasury, shares[3]);
+ if(shares[3] > 0) ITreasury(treasury).deposit(address(raacToken), shares[3]);
}

Modify the FeeCollector::emergencyWithdraw:

function emergencyWithdraw(address token) external override whenPaused {
if (!hasRole(EMERGENCY_ROLE, msg.sender)) revert UnauthorizedCaller();
if (token == address(0)) revert InvalidAddress();
uint256 balance;
if (token == address(raacToken)) {
balance = raacToken.balanceOf(address(this));
- raacToken.safeTransfer(treasury, balance);
+ ITreasury(treasury).deposit(address(raacToken), balance);
} else {
balance = IERC20(token).balanceOf(address(this));
- SafeERC20.safeTransfer(IERC20(token), treasury, balance);
+ ITreasury(treasury).deposit(address(token), balance);
}
emit EmergencyWithdrawal(token, balance);
}
Updates

Lead Judging Commences

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

FeeCollector::_processDistributions and emergencyWithdraw directly transfer funds to Treasury where they get permanently stuck

Support

FAQs

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