Summary
The RAACReleaseOrchestrator::emergencyRevoke function performs an unnecessary self-transfer of tokens that triggers the RAAC token's fee mechanism, resulting in permanent loss of tokens through swap and burn fees.
Vulnerability Details
The emergencyRevoke function contains a flaw where it transfers unreleased tokens to itself:
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);
}
The contract already owns these tokens, so transferring them to itself is unnecessary. However, this self-transfer triggers the RAAC token's fee mechanism:
POC
To use foundry in the codebase, follow the hardhat guide here: Foundry-Hardhat hybrid integration by Nomic foundation
pragma solidity ^0.8.19;
import {FeeCollector} from "../../../../contracts/core/collectors/FeeCollector.sol";
import {RAACToken, PercentageMath} from "../../../../contracts/core/tokens/RAACToken.sol";
import {RAACReleaseOrchestrator} from
"../../../../contracts/core/minters/RAACReleaseOrchestrator/RAACReleaseOrchestrator.sol";
import {veRAACToken} from "../../../../contracts/core/tokens/veRAACToken.sol";
import {Test, console} from "forge-std/Test.sol";
contract TestSuite is Test {
FeeCollector feeCollector;
RAACReleaseOrchestrator orchestrator;
RAACToken raacToken;
veRAACToken veRAACTok;
address treasury;
address repairFund;
address admin;
uint256 initialSwapTaxRate = 100;
uint256 initialBurnTaxRate = 50;
function setUp() public {
treasury = makeAddr("treasury");
repairFund = makeAddr("repairFund");
admin = makeAddr("admin");
raacToken = new RAACToken(admin, initialSwapTaxRate, initialBurnTaxRate);
veRAACTok = new veRAACToken(address(raacToken));
feeCollector = new FeeCollector(address(raacToken), address(veRAACTok), treasury, repairFund, admin);
vm.startPrank(admin);
raacToken.setFeeCollector(address(feeCollector));
raacToken.setMinter(admin);
orchestrator = new RAACReleaseOrchestrator(address(raacToken));
vm.stopPrank();
}
function testEmergencyRevokeTokenLoss() public {
address beneficiary = address(0x1);
uint256 mintAmount = 1000000 ether;
uint256 vestingAmount = 1000 ether;
vm.startPrank(admin);
raacToken.mint(address(orchestrator), mintAmount);
orchestrator.createVestingSchedule(beneficiary, orchestrator.TEAM_CATEGORY(), vestingAmount, block.timestamp);
vm.stopPrank();
uint256 initialBalance = raacToken.balanceOf(address(orchestrator));
vm.prank(admin);
orchestrator.emergencyRevoke(beneficiary);
uint256 finalBalance = raacToken.balanceOf(address(orchestrator));
assertLt(finalBalance, initialBalance, "Balance decreases due to transfer fee");
assertEq(initialBalance - finalBalance, 15 ether, "Should lose exactly 1.5% (15 ether) to fees");
}
}
Impact
Loss of tokens through fees (1.5% of revoked amount). Tokens burnt are permanently lost. More severe with larger vesting amounts due to percentage-based loss.
Tools Used
Manual review, foundry test suite
Recommendations
Remove the unnecessary self-transfer since the contract already owns the tokens:
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);
}
