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

Using `RAACReleaseOrchestrator::emergencyRevoke` would lead to loss of funds and incorrect event emission

tejaswarambhe

Summary

When the RAACReleaseOrchestrator::emergencyRevoke is used, the self transferring of raacToken leads to loss of funds due to tax mechanism and an incorrect event emission.

Vulnerability Details

The RAACReleaseOrchestrator::emergencyRevoke function is design to revoke a beneficiary's vesting when deemed necessary by the admin / emergency role holder.

However, this function unnecessarily transfers tokens to itself, which leads to the contract receiving less raacToken than before due to taxation.
The tax here comprises of burning tokens and transferring to fee collector.

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); <<@ -- // Transfer to ownself which deducts tax
emit EmergencyWithdraw(beneficiary, unreleasedAmount); <<@ -- // Incorrect event is emitted as the actual amount withdrawn is lesser than the `unreleasedAmount` due to taxation
}
emit VestingScheduleRevoked(beneficiary);
}

The event emitted is also incorrect as the emergency withdraw actually withdrew fewer tokens than expected.

Impact

  1. Taxation burns tokens, which is clear loss of funds, however, the funds sent to the fee collector can be considered retrievable but inefficient and sub-optimal.

  2. Actual token holdings of the RAACReleaseOrchestrator contract is reduced, so if the deposits were made to be accurate to the amount allocated, some users may face DoS due to lack of funds.

  3. Incorret event parameters in EmergencyWithdraw event as the amount withdrawn is technically lesser.

Proof of Concept

Create a new file with name RAACReleaseOrchestrator.test.js and run the test case shown below:

import { expect } from "chai";
import { keccak256, toUtf8Bytes } from "ethers";
import hre from "hardhat";
const { ethers } = hre;
​
describe("RAACReleaseOrchestrator", function () {
let RAACToken, RAACReleaseOrchestrator;
let raacToken, releaseOrchestrator;
let owner, user1;
beforeEach(async function () {
[owner, user1] = await ethers.getSigners();
RAACToken = await ethers.getContractFactory("RAACToken");
raacToken = await RAACToken.deploy(owner.address, 100, 50);
RAACReleaseOrchestrator = await ethers.getContractFactory("RAACReleaseOrchestrator");
releaseOrchestrator = await RAACReleaseOrchestrator.connect(user1).deploy(raacToken.target);
await raacToken.setMinter(await user1.getAddress());
// mint to release orchestrator
await raacToken.connect(user1).mint(await releaseOrchestrator.getAddress(), ethers.parseEther("1000000"));
});
​
it('should pay tax on emergency revoke', async function () {
// create vesting schedule
const currentTimestamp = Math.floor(Date.now() / 1000);
await releaseOrchestrator.connect(user1).createVestingSchedule(user1.address, keccak256(toUtf8Bytes("PUBLIC_SALE")), ethers.parseEther('100'), currentTimestamp);
// Check current balance of orchestrator
const orchestratorBalance = await raacToken.balanceOf(await releaseOrchestrator.getAddress());
expect(orchestratorBalance).to.equal(ethers.parseEther('1000000'));
// emergency revoke
await releaseOrchestrator.connect(user1).emergencyRevoke(user1.address);
// Check current balance of orchestrator
const orchestratorBalanceAfterRevoke = await raacToken.balanceOf(await releaseOrchestrator.getAddress());
console.log("orchestratorBalanceAfterRevoke", ethers.formatUnits(orchestratorBalanceAfterRevoke, 18));
// 999998500000000000000000
expect(orchestratorBalanceAfterRevoke).to.be.lessThan(ethers.parseEther('999999'));
});
});

Tools Used

Manual Review
/
Hardhat

Recommendations

It is recommended to remove that transfer 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);
}
Updates

Lead Judging Commences

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

RAACReleaseOrchestrator::emergencyRevoke sends revoked tokens to contract address with no withdrawal mechanism, permanently locking funds

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

RAACReleaseOrchestrator::emergencyRevoke sends revoked tokens to contract address with no withdrawal mechanism, permanently locking funds

Support

FAQs

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