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

EmergencyRevoke Function Fails to Properly Recover Tokens and Corrupts Category Accounting in Token Vesting Contract

lamsy

Summary

The emergencyRevoke function in RAACReleaseOrchestrator contract contains critical flaws in token recovery and category accounting. The function attempts to transfer tokens to itself which will make the protocol incure free on token transfer to itself. It also fails to update category allocation tracking, potentially leading to permanently locked tokens and unusable category allocations.

Vulnerability Details

function emergencyRevoke(address beneficiary) external onlyRole(EMERGENCY_ROLE) {
VestingSchedule storage schedule = vestingSchedules[beneficiary];
if (!schedule.initialized) revert NoVestingSchedule();
uint256 unreleasedAmount = schedule.totalAmount - schedule.releasedAmount;
//@audit no uppdate of category accounting for deleting vestingSchedules
delete vestingSchedules[beneficiary];
if (unreleasedAmount > 0) {
//@audit this is redudant because the contract is the owner of the token.
raacToken.transfer(address(this), unreleasedAmount);
emit EmergencyWithdraw(beneficiary, unreleasedAmount);
}
//@udit event is not detailed enough about revoked vesting
emit VestingScheduleRevoked(beneficiary);
}

The emergencyRevoke function does not reduced the categoryUsed by the amount of the totalAmount which can cause break in the accounting of the protocol. It also add unnecessary transfer of unreleased amount which will make the balance of the contract to reduce because of fee on transfer which will cause loss of fund to the protocol, this should be removed because the contract is the owner of the token or a recovery address can be added to send the unreleaseAmount to (that is if the protocol would still like to transfer the unreleasedAmount out) . The VestingScheduleRevoked event does not fully contain the information of the revoked vesting scheduled, it is meant to have the catergory.

Poc:

import { expect } from "chai";
import hre from "hardhat";
const { ethers } = hre;
import { time } from "@nomicfoundation/hardhat-network-helpers";
​
​
describe.only("RAACReleaseOrchestrator Emergency Revoke", function () {
let releaseOrchestrator, raacToken;
let owner, beneficiary, emergencyRole;
const TEAM_CATEGORY = ethers.keccak256(ethers.toUtf8Bytes("TEAM"));
const VESTING_AMOUNT = ethers.parseEther("1000000");
const VESTING_CLIFF = 90 * 24 * 3600; // 90 days
​
beforeEach(async function () {
[owner, beneficiary, emergencyRole] = await ethers.getSigners();
​
// Deploy and setup contracts
raacToken = await ethers.deployContract("RAACToken", [
owner.address,
100,
50,
]);
releaseOrchestrator = await ethers.deployContract(
"RAACReleaseOrchestrator",
[await raacToken.getAddress()]
);
​
// Setup roles and initial token balance
await releaseOrchestrator.grantRole(
await releaseOrchestrator.EMERGENCY_ROLE(),
emergencyRole.address
);
await raacToken.setMinter(owner.address);
await raacToken.mint(
await releaseOrchestrator.getAddress(),
ethers.parseEther("65000000")
);
});
​
describe("Emergency Revoke Issues", function () {
it("should demonstrate emergency revoke issues when no vestiing has been released", async function () {
const blockTimestamp = (await ethers.provider.getBlock("latest"))
.timestamp;
await releaseOrchestrator.createVestingSchedule(
beneficiary.address,
TEAM_CATEGORY,
VESTING_AMOUNT,
blockTimestamp
);
​
// Record initial states
const initialBalance = await raacToken.balanceOf(
await releaseOrchestrator.getAddress()
);
const initialCategoryUsed = await releaseOrchestrator.categoryUsed(
TEAM_CATEGORY
);
​
// 2. Perform emergency revoke
await releaseOrchestrator
.connect(emergencyRole)
.emergencyRevoke(beneficiary.address);
​
// 3. Verify issues
expect(
await raacToken.balanceOf(await releaseOrchestrator.getAddress())
).to.equal(
initialBalance,
"Contract balance was reduced because of fee on transfer to self"
);
});
it("should demonstrate emergency revoke issues when vestiing has been released but later revoked ", async function () {
const blockTimestamp = (await ethers.provider.getBlock("latest"))
.timestamp;
const vestAmount = ethers.parseEther("1000000");
console.log(
"releaseOrchestrator balance before",
await raacToken.balanceOf(await releaseOrchestrator.getAddress())
);
// 1. Create vesting schedule
await releaseOrchestrator.createVestingSchedule(
beneficiary.address,
TEAM_CATEGORY,
vestAmount,
blockTimestamp
);
​
// 2. Move past cliff period and do initial release
await network.provider.send("evm_increaseTime", [VESTING_CLIFF + 1]);
await network.provider.send("evm_mine");
​
// 3. Beneficiary claims tokens
await releaseOrchestrator.connect(beneficiary).release();
const balanceOfBeneficiary = await raacToken.balanceOf(
beneficiary.address
);
​
// 4. Record states before emergency revoke
const releasedAmount = await releaseOrchestrator.getVestingSchedule(
beneficiary.address
);
​
console.log(
"transfer fee : ",
(releasedAmount.releasedAmount.toString() -
balanceOfBeneficiary.toString()) /
1e18
);
​
const initialCategoryUsed = await releaseOrchestrator.categoryUsed(
TEAM_CATEGORY
);
​
const OrchestratorBalanceAfter = await raacToken.balanceOf(
await releaseOrchestrator.getAddress()
);
console.log(
"\n balance of the orchestrator after first release",
OrchestratorBalanceAfter.toString()
);
// 5. Perform emergency revoke
await releaseOrchestrator
.connect(emergencyRole)
.emergencyRevoke(beneficiary.address);
​
console.log(
"getCategoryDetails ",
await releaseOrchestrator.getCategoryDetails(TEAM_CATEGORY)
);
// 6. Verify states after revoke
// Category accounting should be reduced by unreleased amount
const expectedCategoryUsage = initialCategoryUsed - vestAmount;
expect(await releaseOrchestrator.categoryUsed(TEAM_CATEGORY)).to.equal(
expectedCategoryUsage,
"Category usage should be reduced by released amount"
);
});
});
});
​

Outcome:

​
​
RAACReleaseOrchestrator Emergency Revoke
Emergency Revoke Issues
1) should demonstrate emergency revoke issues when no vestiing has been released
releaseOrchestrator balance before 65000000000000000000000000n
transfer fee : 1928.5721726190532
​
balance of the orchestrator after first release 64871428521825396825396826
getCategoryDetails Result(2) [ 18000000000000000000000000n, 1000000000000000000000000n ]
2) should demonstrate emergency revoke issues when vestiing has been released but later revoked
​
​
0 passing (2s)
2 failing
​
1) RAACReleaseOrchestrator Emergency Revoke
Emergency Revoke Issues
should demonstrate emergency revoke issues when no vestiing has been released:
​
Contract balance was reduced because of fee on transfer to self
+ expected - actual
​
-64985000000000000000000000
+65000000000000000000000000
2) RAACReleaseOrchestrator Emergency Revoke
Emergency Revoke Issues
should demonstrate emergency revoke issues when vestiing has been released but later revoked :
​
Category usage should be reduced by released amount
+ expected - actual
​
-1000000000000000000000000
+0
​

All the test failed because of the issue in the contract.
the contract balance for the first test is meant to be +65000000000000000000000000 because no vesting was release but because of the transfer done raacToken.transfer(address(this), unreleasedAmount); the protocol incure some lose due to fee on transfer of RAACToken

Impact:

  • Category allocations remain permanently allocated which will cause accumulation of categoryUsed despite amount not released this may cause total vesting capacity reduced in the categoryUsed which will lead to improper categoryUsed[category] accounting.

    uint256 newCategoryTotal = categoryUsed[category] + amount;
    if (newCategoryTotal > categoryAllocations[category]) revert CategoryAllocationExceeded();
    categoryUsed[category] = newCategoryTotal;
    ​

  • Loss of funds to the protocol due to transfering its own token to itself which is unneccessary

Tools Used

Manual review

Recommendations

  • Either add a recovery address to the contract to transfer all unreleasedAmount or add a state variable that will track unreleasedAmount vesting

  • Also reduce the categoryUsed[category] by the schedule.totalAmount in the emergencyRevoke

function emergencyRevoke(
address beneficiary,
address recoveryAddress
) external onlyRole(EMERGENCY_ROLE) {
if (recoveryAddress == address(0)) revert InvalidRecoveryAddress();
VestingSchedule storage schedule = vestingSchedules[beneficiary];
if (!schedule.initialized) revert NoVestingSchedule();
uint256 unreleasedAmount = schedule.totalAmount - schedule.releasedAmount;
+ bytes32 category = schedule.category; // Add category to VestingSchedule struct
// Update category accounting
+ categoryUsed[category] -= schedule.totalAmount;
// Transfer to recovery address
if (unreleasedAmount > 0) {
+ raacToken.transfer(recoveryAddress, unreleasedAmount);
emit EmergencyWithdraw(beneficiary, recoveryAddress, unreleasedAmount);
}
delete vestingSchedules[beneficiary];
emit VestingScheduleRevoked(beneficiary, category);
}
struct VestingSchedule {
uint256 totalAmount;
uint256 startTime;
uint256 duration;
uint256 lastClaimTime;
uint256 releasedAmount;
bool initialized;
bytes32 category; // Add category for proper accounting
}
event EmergencyWithdraw(
address indexed beneficiary,
address indexed recoveryAddress,
uint256 amount
);
event VestingScheduleRevoked(
address indexed beneficiary,
address indexed category
);

OR

​
struct RevokedVestingSchedule {
address beneficiary;
bytes32 category;
uint256 unreleasedAmount;
}
​
mapping(address => RevokedVestingSchedule) public revokedVestingSchedules;
​
​
function emergencyRevoke(
address beneficiary
) external onlyRole(EMERGENCY_ROLE) {
if (recoveryAddress == address(0)) revert InvalidRecoveryAddress();
VestingSchedule storage schedule = vestingSchedules[beneficiary];
if (!schedule.initialized) revert NoVestingSchedule();
uint256 unreleasedAmount = schedule.totalAmount - schedule.releasedAmount;
+ bytes32 category = schedule.category; // Add category to VestingSchedule struct
// Update category accounting
+ categoryUsed[category] -= schedule.totalAmount;
// store in a RevokedVestingSchedule to track amount not released if the protocol does not need to send it
if (unreleasedAmount > 0) {
+ RevokedVestingSchedule storage revokedvesting = revokedVestingSchedules[beneficiary];
+ revokedvesting.beneficiary = beneficiary;
+ revokedvesting.category= schedule.category;
+ revokedvesting.category= unreleasedAmount;
}
delete vestingSchedules[beneficiary];
emit VestingScheduleRevoked(beneficiary, category);
}
​
event VestingScheduleRevoked(
address indexed beneficiary,
address indexed category
);
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 month ago
Submission Judgement Published
Validated
Assigned finding tags:

RAACReleaseOrchestrator::emergencyRevoke fails to decrement categoryUsed, causing artificial category over-allocation and rejection of valid vesting schedules

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

inallhonesty Lead Judge about 1 month ago
Submission Judgement Published
Validated
Assigned finding tags:

RAACReleaseOrchestrator::emergencyRevoke fails to decrement categoryUsed, causing artificial category over-allocation and rejection of valid vesting schedules

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.