Core Contracts

Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Submission Details
Severity: high
Invalid

Self-Transfer in emergencyRevoke Causes Token Loss Due to Transfer Taxes

Summary

The emergencyRevoke function in the RAACReleaseOrchestrator contract is designed to revoke a beneficiary's vesting schedule in emergency scenarios. Although the function correctly updates storage by deleting the vesting schedule and emits the proper revocation event, it performs a self-transfer of unreleased tokens from the contract to itself. Since the RAACToken contract applies transfer taxes on every token transfer, this self-transfer causes a tax deduction, resulting in a reduction of the contract's RAAC token balance. Over time, such inadvertent token losses can lead to a denial of service (DoS) where the orchestrator may not have sufficient tokens to manage vesting schedules or execute other critical functions.

Vulnerability Details

How It Arises

  • Self-Transfer in emergencyRevoke:

    The problematic code in emergencyRevoke is as follows:

    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) {
    // Self-transfer from the contract to itself triggers transfer taxes
    raacToken.transfer(address(this), unreleasedAmount);
    emit EmergencyWithdraw(beneficiary, unreleasedAmount);
    }
    emit VestingScheduleRevoked(beneficiary);
    }

    Here, the contract transfers tokens from itself to itself in order to "revoke" the unreleased tokens. However, the RAACToken contract charges taxes on every transfer, including self-transfers. As a result, a portion of the tokens is deducted as tax during this operation, reducing the overall RAAC balance held by the RAACReleaseOrchestrator contract.

  • Impact on Token Balance:

    Due to the tax deductions during self-transfer, the contract loses tokens that should have remained available. This unintended token loss can eventually cause a critical shortage of RAAC tokens in the orchestrator, potentially leading to operational failures in managing vesting schedules or executing further token releases.

Proof of Concept

Scenario Walkthrough

  1. Setup:

    • A vesting schedule is created for a beneficiary (e.g., ALICE) with a specified allocation.

    • Tokens are minted to the RAACReleaseOrchestrator to fund the vesting schedule.

  2. Emergency Revoke Execution:

    • The orchestrator calls emergencyRevoke(ALICE).

    • The function calculates the unreleasedAmount and performs a self-transfer to itself.

    • Due to the token transfer taxes, the actual tokens transferred are less than unreleasedAmount.

  3. Observation:

    • A test logs the RAAC token balance of the RAACReleaseOrchestrator before and after the emergency revoke.

    • The balance after the revoke is lower, confirming that tokens were lost due to tax deductions.

Test Suite PoC

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
import {Test, console} from "forge-std/Test.sol";
import {RAACToken} from "../src/core/tokens/RAACToken.sol";
import {RAACReleaseOrchestrator} from "../src/core/minters/RAACReleaseOrchestrator/RAACReleaseOrchestrator.sol";
contract RAACReleaseOrchestratorTest is Test {
RAACToken raacToken;
RAACReleaseOrchestrator raacReleaseOrchestrator;
address RAAC_OWNER = makeAddr("RAAC_OWNER");
address RAAC_MINTER = makeAddr("RAAC_MINTER");
address RAAC_ORCHESTRATOR_OWNER = makeAddr("RAAC_ORCHESTRATOR_OWNER");
uint256 initialRaacSwapTaxRateInBps = 200; // 2%
uint256 initialRaacBurnTaxRateInBps = 150; // 1.5%
address ALICE = makeAddr("ALICE");
address BOB = makeAddr("BOB");
address CHARLIE = makeAddr("CHARLIE");
address DEVIL = makeAddr("DEVIL");
event EmergencyWithdraw(address indexed beneficiary, uint256 amount);
function setUp() public {
raacToken = new RAACToken(RAAC_OWNER, initialRaacSwapTaxRateInBps, initialRaacBurnTaxRateInBps);
vm.startPrank(RAAC_ORCHESTRATOR_OWNER);
raacReleaseOrchestrator = new RAACReleaseOrchestrator(address(raacToken));
vm.stopPrank();
}
function testSelfTransferOnEmergencyRevokeImpactsRaacTokenBalance() public {
bytes32 category = raacReleaseOrchestrator.TREASURY_CATEGORY();
uint256 categoryAllocation = raacReleaseOrchestrator.categoryAllocations(category);
vm.startPrank(RAAC_OWNER);
raacToken.setMinter(RAAC_MINTER);
vm.stopPrank();
vm.startPrank(RAAC_MINTER);
raacToken.mint(address(raacReleaseOrchestrator), categoryAllocation);
vm.stopPrank();
uint256 vestingDuration = raacReleaseOrchestrator.VESTING_DURATION();
vm.startPrank(RAAC_ORCHESTRATOR_OWNER);
raacReleaseOrchestrator.createVestingSchedule(ALICE, category, categoryAllocation, block.timestamp);
vm.stopPrank();
// Advance time so that vesting is not complete.
vm.warp(block.timestamp + vestingDuration / 4);
uint256 balanceBefore = raacToken.balanceOf(address(raacReleaseOrchestrator));
console.log("Balance before emergency revoke:", balanceBefore);
// Execute emergency revoke, which performs a self-transfer.
vm.startPrank(RAAC_ORCHESTRATOR_OWNER);
raacReleaseOrchestrator.emergencyRevoke(ALICE);
vm.stopPrank();
uint256 balanceAfter = raacToken.balanceOf(address(raacReleaseOrchestrator));
console.log("Balance after emergency revoke:", balanceAfter);
// The balance after should be less than the balance before due to tax deductions.
assertNotEq(balanceAfter, balanceBefore);
}
}

How to Run the Test

  1. Initialize a Foundry Project:

    forge init my-foundry-project
  2. Place Contract Files:
    Ensure that RAACToken.sol and RAACReleaseOrchestrator.sol are correctly located under src/core/tokens and src/core/minters/RAACReleaseOrchestrator respectively.

  3. Create Test Directory:
    Create a test directory adjacent to src and add the test file (e.g., RAACReleaseOrchestratorTest.t.sol).

  4. Run the Test:

    forge test --mt testSelfTransferOnEmergencyRevokeImpactsRaacTokenBalance -vv
  5. Expected Outcome:
    The RAACReleaseOrchestrator’s RAAC token balance will decrease after calling emergencyRevoke due to tax deductions on the self-transfer, proving the vulnerability.

Impact

  • Token Loss:
    Self-transfers incur transfer taxes, causing the RAACReleaseOrchestrator contract to lose tokens. Over time, repeated self-transfers can deplete the contract’s token balance.

  • Denial of Service:
    Reduced token balances may hinder the contract’s ability to perform subsequent vesting operations or fulfill other financial obligations.

  • Economic Disruption:
    Unintended token loss affects the overall tokenomics, potentially leading to an imbalance between the total tokens minted and those available for distribution.

Tools Used

  • Manual Review

  • Foundry

Recommendations

To prevent unnecessary token loss, the emergencyRevoke function should not perform a self-transfer of unreleased tokens. Instead, it should simply revoke the vesting schedule and emit the appropriate event without altering the token balance.

Proposed Diff for emergencyRevoke Function

function emergencyRevoke(address beneficiary) external onlyRole(EMERGENCY_ROLE) {
- VestingSchedule storage schedule = vestingSchedules[beneficiary];
- if (!schedule.initialized) revert NoVestingSchedule();
+ // Verify that the vesting schedule exists before revoking.
+ if (!vestingSchedules[beneficiary].initialized) revert NoVestingSchedule();
- uint256 unreleasedAmount = schedule.totalAmount - schedule.releasedAmount;
+ // Revoke the vesting schedule without performing a self-transfer.
delete vestingSchedules[beneficiary];
-
- if (unreleasedAmount > 0) {
- raacToken.transfer(address(this), unreleasedAmount);
- emit EmergencyWithdraw(beneficiary, unreleasedAmount);
- }
emit VestingScheduleRevoked(beneficiary);
}

Benefits of This Approach

  • Prevents Token Loss:
    Eliminates self-transfers, thus avoiding tax deductions and preserving the RAAC token balance within the contract.

  • Simplified Logic:
    The function becomes minimal and focused solely on revoking the vesting schedule, reducing the potential for errors.

  • Maintained Integrity:
    By not altering the token balance unnecessarily, the contract ensures that its accounting remains accurate, which is crucial for subsequent operations and audits.

Implementing these changes will prevent unnecessary token loss due to transfer taxes during emergency revokes, ensuring that the RAACReleaseOrchestrator contract maintains an accurate token balance while revoking vesting schedules as intended. This correction will help maintain the protocol's economic integrity and operational reliability.

Updates

Lead Judging Commences

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!