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

Missing allocations update when revoking unreleased vesting tokens prevent creation of vesting schedules

r4bbit

Summary

The RAACReleaseOrchestrator allows for revoking vesting allocations from beneficiaries using emergencyRevoke. The unreleased tokens are returned to the protocol, but due to a missing update in the used category allocation, this can prevent the creation of new vesting schedules.

Vulnerability Details

The RAACReleaseOrchestrator comes with an emergencyRevoke() function that allows for revoking a beneficiary's vesting allocation. The contract calculates the amount of unreleased tokens and transfers them (back) to the contract:

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); // <-- unreleased tokens transferred here
emit EmergencyWithdraw(beneficiary, unreleasedAmount);
}
emit VestingScheduleRevoked(beneficiary);
}

First of all, it should be noted that, the contract already assumes that the tokens for vesting schedules live in the contract. When the contract is initialized, it sets up the various allocations for different categories, and later, when beneficiaries release their tokens from their vesting schedule, the contract sends them to them:

function release() external nonReentrant whenNotPaused {
address beneficiary = msg.sender;
VestingSchedule storage schedule = vestingSchedules[beneficiary];
if (!schedule.initialized) revert NoVestingSchedule();
uint256 releasableAmount = _calculateReleasableAmount(schedule);
if (releasableAmount == 0) revert NothingToRelease();
schedule.releasedAmount += releasableAmount;
schedule.lastClaimTime = block.timestamp;
raacToken.transfer(beneficiary, releasableAmount); // <-- This only works if the contract already owns the tokens
emit TokensReleased(beneficiary, releasableAmount);
}

So, if we assume that the RAACReleaseOrchestrator already owns the necessary RAACToken funds for the configured allocations, then we don't need to perform an explicit raacToken.transfer(address(this), unreleasedAmount) like it's done in emergencyRevoke().

In any case, what's more interesting is that, after revoking allocations from a vesting schedule, the categoryUsed mapping is not updated accordingly. This results in incorrect accounting for the token allocation and could prevent the protocol from creating new vesting schedules.

Notice that, when creating a vesting schedule via createVestingSchedule(), the protocol keeps track of how much allocation is "in use" for a certain category:

function createVestingSchedule(
address beneficiary,
bytes32 category,
uint256 amount,
uint256 startTime
) external onlyRole(ORCHESTRATOR_ROLE) whenNotPaused {
if (beneficiary == address(0)) revert InvalidAddress();
if (amount == 0) revert InvalidAmount();
if (vestingSchedules[beneficiary].initialized) revert VestingAlreadyInitialized();
if (categoryAllocations[category] == 0) revert InvalidCategory();
// Check category allocation limits
uint256 newCategoryTotal = categoryUsed[category] + amount;
if (newCategoryTotal > categoryAllocations[category]) revert CategoryAllocationExceeded(); // <-- reverts if category allocation is exceeded
categoryUsed[category] = newCategoryTotal;
VestingSchedule storage schedule = vestingSchedules[beneficiary];
schedule.totalAmount = amount;
schedule.startTime = startTime;
schedule.duration = VESTING_DURATION;
schedule.initialized = true;
emit VestingScheduleCreated(beneficiary, category, amount, startTime);
}

Given that emergencyRevoke() doesn't free the unreleased tokens from categoryUsed[category], the protocol thinks that more allocation is in use than what's actually the case.

One way to get around this is to use updateCategoryAllocation() to increase the category allocation, however, this would only skew the numbers and would probably force the owner of the protocol to also make that new allocation actually available to the protocol (meaning, the increase in allocation also needs to be reflected in the contract's token balance).

Impact

As described above, it's possible to "free up" that allocation at the cost of virtually increasing the overall category allocation, but the reported numbers would still reflect incorrect data. So unless this is tackled, it could theoretically prevent the protocol from creating new vesting schedules.

Tools Used

Manual review.

Recommendations

Ensure categoryUsed[category] is reduced in lockstep when allocations are revoked from vesting schedules.
Also, reconsider if the revoked tokens should actually in the orchestrator contract, or if they should go somewhere else to be reallocated.

Here are the necessary changes.

First of all, VestingSchedule needs to keep track of the category:

interface IReleaseOrchestrator {
/**
* @notice Struct to track vesting schedule for each beneficiary
*/
struct VestingSchedule {
uint256 totalAmount; // Total amount allocated
uint256 releasedAmount; // Amount already released
uint256 startTime; // Start time of vesting
uint256 duration; // Duration of vesting in seconds
uint256 lastClaimTime; // Last time tokens were claimed
bool initialized; // Whether schedule is initialized
+. bytes32 category;
}
...
}

Then, when the vesting schedule is created, make sure to add the category to the data:

function createVestingSchedule(
address beneficiary,
bytes32 category,
uint256 amount,
uint256 startTime
) external onlyRole(ORCHESTRATOR_ROLE) whenNotPaused {
if (beneficiary == address(0)) revert InvalidAddress();
if (amount == 0) revert InvalidAmount();
if (vestingSchedules[beneficiary].initialized) revert VestingAlreadyInitialized();
if (categoryAllocations[category] == 0) revert InvalidCategory();
// Check category allocation limits
uint256 newCategoryTotal = categoryUsed[category] + amount;
if (newCategoryTotal > categoryAllocations[category]) revert CategoryAllocationExceeded();
categoryUsed[category] = newCategoryTotal;
VestingSchedule storage schedule = vestingSchedules[beneficiary];
schedule.totalAmount = amount;
schedule.startTime = startTime;
schedule.duration = VESTING_DURATION;
+. schedule.category = category;
schedule.initialized = true;
emit VestingScheduleCreated(beneficiary, category, amount, startTime);
}
​

Lastly, when revoking tokens, reduce the allocation in categoryUsed by the same amount:

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) {
+ categoryUsed[schedule.category] = categoryUsed[schedule.category] - unreleasedAmount;
raacToken.transfer(address(this), unreleasedAmount);
emit EmergencyWithdraw(beneficiary, unreleasedAmount);
}
+ delete vestingSchedules[beneficiary];
emit VestingScheduleRevoked(beneficiary);
}
​

Relevant links

Updates

Lead Judging Commences

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

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

Support

FAQs

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

Give us feedback!