Summary
The emergencyRevoke() function unnecessarily transfers tokens to itself, which triggers the RAAC token's transfer fee mechanism if the contract is not whitelisted, permanently reducing the amount available for future beneficiaries.
Vulnerability Details
In RAACReleaseOrchestrator::emergencyRevoke() function performs a self-transfer of unreleased tokens:
function emergencyRevoke(address beneficiary) external onlyRole(EMERGENCY_ROLE) {
if (unreleasedAmount > 0) {
raacToken.transfer(address(this), unreleasedAmount);
emit EmergencyWithdraw(beneficiary, unreleasedAmount);
}
}
This self-transfer is unnecessary since the tokens are already held by the contract. More critically, if the orchestrator contract is not whitelisted in the RAAC token contract, this transfer will incur fees, permanently reducing the token amount available for future beneficiaries.
Please note that the only way to skip the transfer fee is to be whitelisted in the RAAC token contract. However, in the documentation it does not mention that the orchestrator contract will be whitelisted. Moreover, the deployment script does not whitelist the orchestrator contract. Here is the whitelis setup in the deployment script in the scripts/deploy/03_setup_parameters.js file:
const whitelistAddresses = [
veRAACTokenAddress,
feeCollectorAddress,
treasuryAddress
];
for (const address of whitelistAddresses) {
await raacToken.manageWhitelist(address, true);
console.log(`Whitelisted: ${address}`);
}
So, if is not whitelisted during deployment, the only way would be to whitelist it after deployment. This would require invoking the RAACToken::manageWhitelist() function which is only callable by the owner that is the RAACMinter contract. However, the RAACMinter contract does not have a function call RAACToken::manageWhitelist().
Impact
When emergency revocation occurs, a percentage of tokens are permanently lost due to transfer fees, directly reducing the amount available for reassignment to new beneficiaries. This creates a loss of assets that cannot be recovered and prevents users from being able to release their tokens.
Tools Used
Manual review
Proof of Concept
Create a new file test/unit/core/minters/RAACReleaseOrchestrator.test.js and add the following test case:
import { expect } from "chai";
import hre from "hardhat";
const { ethers } = hre;
import { time } from "@nomicfoundation/hardhat-network-helpers";
describe("RAACReleaseOrchestrator", function () {
it('shows token loss during emergency revoke due to transfer fees', async function () {
const [owner, user1, user2] = await ethers.getSigners();
const RAACTokenFactory = await ethers.getContractFactory("RAACToken");
const raacToken = await RAACTokenFactory.deploy(owner.address, 0, 0);
await raacToken.connect(owner).setMinter(owner.address);
const RAACReleaseOrchestratorFactory = await ethers.getContractFactory("RAACReleaseOrchestrator");
const raacReleaseOrchestrator = await RAACReleaseOrchestratorFactory.deploy(await raacToken.getAddress());
const TEAM_CATEGORY = await raacReleaseOrchestrator.TEAM_CATEGORY();
const vestingAmount = ethers.parseEther("9000000");
const startTime = await time.latest();
await raacReleaseOrchestrator.connect(owner).createVestingSchedule(user1.address, TEAM_CATEGORY, vestingAmount, startTime);
await raacToken.connect(owner).mint(raacReleaseOrchestrator.target, vestingAmount);
const VESTING_DURATION = await raacReleaseOrchestrator.VESTING_DURATION();
await time.increase(VESTING_DURATION / 2n);
await raacReleaseOrchestrator.connect(owner).emergencyRevoke(user1.address);
const user1Balance = await raacToken.balanceOf(user1.address);
expect(user1Balance).to.equal(0);
const orchestratorBalance = await raacToken.balanceOf(raacReleaseOrchestrator.target);
expect(orchestratorBalance).to.lt(vestingAmount);
await raacReleaseOrchestrator.connect(owner).createVestingSchedule(user2.address, TEAM_CATEGORY, vestingAmount, startTime);
await time.increase(VESTING_DURATION);
await expect(raacReleaseOrchestrator.connect(user2).release()).to.be.revertedWithCustomError(raacToken, "ERC20InsufficientBalance");
});
});
And run the test with:
npx hardhat test test/unit/core/minters/RAACReleaseOrchestrator.test.js
Recommendations
Remove the unnecessary self-transfer in the emergencyRevoke() function:
function emergencyRevoke(address beneficiary) external onlyRole(EMERGENCY_ROLE) {
VestingSchedule storage schedule = vestingSchedules[beneficiary];
if (!schedule.initialized) revert NoVestingSchedule();
uint256 unreleasedAmount = schedule.totalAmount - schedule.releasedAmount;
delete vestingSchedules[beneficiary];
if (unreleasedAmount > 0) {
- raacToken.transfer(address(this), unreleasedAmount);
emit EmergencyWithdraw(beneficiary, unreleasedAmount);
}
emit VestingScheduleRevoked(beneficiary);
}
