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

`FeeCollector::emergencyWithdraw` Function Causes Permanent Fund Locking in `Treasury`

federodes

Summary

The FeeCollector::emergencyWithdraw function directly transfers tokens to the Treasury contract without updating its internal _balances mapping. As a result, the Treasury contract does not reflect the actual token balance, which leads to funds being permanently locked and unrecoverable.

Vulnerability Details

The emergencyWithdraw function allows an authorized caller to withdraw tokens from the FeeCollector contract and send them to the Treasury. However, instead of using the deposit function in the Treasury contract, it transfers tokens directly using safeTransfer. Since Treasury tracks balances manually in _balances, this leads to a discrepancy where _balances[token] remains zero while the actual token balance is nonzero.

As a result, any subsequent calls to withdraw in the Treasury contract will fail due to the InsufficientBalance check, effectively locking the tokens inside the contract.

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);
// @audit-issue this call transfers tokens to the Treasury contract
// directly via the transfer function instead of the intended Treasury::deposit
} else {
balance = IERC20(token).balanceOf(address(this));
@> SafeERC20.safeTransfer(IERC20(token), treasury, balance);
}
​
emit EmergencyWithdrawal(token, balance);
}
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;
// @audit-issue balance is only tracked when using the deposit function.
// Assets transferred via transfer will not get tracked.
_totalValue += amount;
​
emit Deposited(token, amount);
}
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(); // Always fails
​
@> _balances[token] -= amount;
_totalValue -= amount;
IERC20(token).transfer(recipient, amount);
​
emit Withdrawn(token, amount, recipient);
}

Issue:

  • _balances[token] remains zero after emergencyWithdraw, making every withdrawal attempt revert with InsufficientBalance().

  • The actual token balance is nonzero, but it cannot be accessed due to the incorrect _balances mapping.

PoC

Create a new file named FeeCollectorTreasury inside test/unit/core/collectors, then paste the test content into it. Finally, run the test using the following command:

npx hardhat test test/unit/core/collectors/FeeCollectorTreasury.test.js
import { expect } from "chai";
import hre from "hardhat";
const { ethers } = hre;
​
describe("FeeCollectorTreasury", function () {
let raacToken, feeCollector, veRAACToken;
let owner, user1, user2, manager, allocator, treasury, newTreasury, repairFund, emergencyAdmin;
​
const INITIAL_MINT = ethers.parseEther("10000");
const SWAP_TAX_RATE = 100; // 1%
const BURN_TAX_RATE = 50; // 0.5%
​
const MANAGER_ROLE = ethers.keccak256(ethers.toUtf8Bytes("MANAGER_ROLE"));
const ALLOCATOR_ROLE = ethers.keccak256(ethers.toUtf8Bytes("ALLOCATOR_ROLE"));
​
beforeEach(async function () {
[owner, user1, user2, manager, allocator, newTreasury, 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);
​
// Setup roles
await treasury.grantRole(MANAGER_ROLE, manager.address);
await treasury.grantRole(ALLOCATOR_ROLE, allocator.address);
​
// Deploy FeeCollector
const FeeCollector = await ethers.getContractFactory("FeeCollector");
​
feeCollector = await FeeCollector.deploy(
await raacToken.getAddress(),
await veRAACToken.getAddress(),
treasury.target,
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);
});
​
describe("Emergency Controls", function () {
beforeEach(async function () {
await feeCollector.connect(owner).grantRole(await feeCollector.EMERGENCY_ROLE(), owner.address);
await raacToken.connect(user1).transfer(feeCollector.target, ethers.parseEther("100"));
});
​
it.only("should allow emergency withdrawal by admin and withdrawal from Treasury", async function () {
await feeCollector.connect(owner).pause();
​
const balanceTreasuryBefore = await raacToken.balanceOf(treasury.target)
​
const amount = ethers.parseEther("100");
​
// Calculate expected amount after two tax applications
await raacToken.connect(user1).transfer(feeCollector.target, amount);
​
await feeCollector.connect(owner).emergencyWithdraw(raacToken.target);
​
const balanceTreasuryAfter = await raacToken.balanceOf(treasury.target);
expect(balanceTreasuryAfter).to.be.gt(balanceTreasuryBefore)
​
// Attempting to withdraw from Treasury should fail
await expect(
treasury.connect(manager).withdraw(raacToken.target, balanceTreasuryAfter, user2.address)
).to.be.revertedWithCustomError(treasury, "InsufficientBalance");
});
});
});

Impact

This vulnerability results in permanent loss of funds, as tokens sent through emergencyWithdraw cannot be withdrawn later. Users and protocol managers will be unable to retrieve funds stored in the Treasury, leading to financial losses and disruptions in protocol operations.

Tools Used

Manual review

Recommendations

Avoid maintaining internal balance tracking; instead, rely on IERC20(token).balanceOf() to determine token holdings. Alternatively, ensure that funds transferred from the FeeCollector to the Treasury contract are processed through the deposit function to keep balances accurately recorded.

Updates

Lead Judging Commences

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

Give us feedback!