Summary
The RAACReleaseOrchestrator::emergencyRevoke function is a critical emergency control mechanism in the that allows authorized parties (EMERGENCY_ROLE) to forcibly terminate a beneficiary's vesting schedule. The function transfers raacToken to address(this) (the contract itself) when revoking a vesting schedule and deletes the vesting. Since the tokens are already held by the contract, this operation is both unnecessary and detrimental as it effectively locks the tokens in the contract without any mechanism to retrieve them.
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;
delete vestingSchedules[beneficiary];
if (unreleasedAmount > 0) {
@> raacToken.transfer(address(this), unreleasedAmount);
emit EmergencyWithdraw(beneficiary, unreleasedAmount);
}
emit VestingScheduleRevoked(beneficiary);
}
Impact
During the RAACReleaseOrchestrator::emergencyRevoke the raacToken are transferred back to address(this)the contract itself and thebeneficiary` vesting is deleted. The tokens become permanently locked in the contract as there's no mechanism to recover them after transfer. This leads to loss of funds for beneficiaries whose vesting schedules are revoked.
Add this line of code raacToken.safeTransferFrom(beneficiary, address(this), amount); in the RAACReleaseOrchestrator::createVesting after the line 91. This fix the missing token transfer on the createVesting function vulnerability already submitted with a separate submission.
Add Foundry to the project following this procedure
Create a file named RAACReleaseOrchestrator.t.sol and copy/paste this:
pragma solidity ^0.8.0;
import {Test, console2} from "forge-std/Test.sol";
import {RAACToken} from "../contracts/core/tokens/RAACToken.sol";
import {RAACReleaseOrchestrator} from "../contracts/core/minters/RAACReleaseOrchestrator/RAACReleaseOrchestrator.sol";
contract RAACReleaseOrchestratorTest is Test {
RAACToken public raacToken;
RAACReleaseOrchestrator public raactReleaseOrchestrator;
address raacTokenOwner = makeAddr("raacTokenOwner");
address beneficiary = makeAddr("beneficiary");
uint256 SWAP_TAX_RATE = 100;
uint256 BURN_TAX_RATE = 50;
function setUp() public {
raacToken = new RAACToken(raacTokenOwner, SWAP_TAX_RATE, BURN_TAX_RATE);
raactReleaseOrchestrator = new RAACReleaseOrchestrator(address(raacToken));
console2.log("raacToken: ", address(raacToken));
console2.log("raactReleaseOrchestrator", address(raactReleaseOrchestrator));
vm.prank(raacTokenOwner);
raacToken.setMinter(address(this));
raacToken.mint(address(raactReleaseOrchestrator), 1000 ether);
raacToken.mint(address(beneficiary), 1000 ether);
}
function test_wrongEmergencyAddressTransfer() public {
assertEq(raacToken.balanceOf(beneficiary), 1000 ether);
assertEq(raacToken.balanceOf(address(raactReleaseOrchestrator)), 1000 ether);
vm.prank(beneficiary);
raacToken.approve(address(raactReleaseOrchestrator), type(uint256).max);
raactReleaseOrchestrator.createVestingSchedule(
beneficiary, raactReleaseOrchestrator.TEAM_CATEGORY(), 1000 ether, block.timestamp
);
assertEq(raacToken.balanceOf(beneficiary), 0 ether);
assertEq(raacToken.balanceOf(address(raactReleaseOrchestrator)), 1985 ether);
raactReleaseOrchestrator.emergencyRevoke(beneficiary);
assertEq(raacToken.balanceOf(beneficiary), 0 ether);
assertEq(raacToken.balanceOf(address(raactReleaseOrchestrator)), 1970 ether);
}
}
Run forge test --match-test test_wrongEmergencyAddressTransfer -vv
Logs:
raacToken: 0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f
raactReleaseOrchestrator 0x2e234DAe75C793f67A35089C9d99245E1C58470b
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 6.57ms (902.00µs CPU time)
The test shows that no tokens are transferred to the beneficiary.
Tools Used
Manual review
Recommendations
Modify the function to transfer tokens to either the beneficiary .
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);
+ raacToken.transfer(beneficiary, unreleasedAmount);
emit EmergencyWithdraw(beneficiary, unreleasedAmount);
}
emit VestingScheduleRevoked(beneficiary);
}
Now copy/past this test:
function test_wrongEmergencyAddressTransferMitigatio() public {
assertEq(raacToken.balanceOf(beneficiary), 1000 ether);
assertEq(raacToken.balanceOf(address(raactReleaseOrchestrator)), 1000 ether);
vm.prank(beneficiary);
raacToken.approve(address(raactReleaseOrchestrator), type(uint256).max);
raactReleaseOrchestrator.createVestingSchedule(
beneficiary, raactReleaseOrchestrator.TEAM_CATEGORY(), 1000 ether, block.timestamp
);
assertEq(raacToken.balanceOf(beneficiary), 0 ether);
assertEq(raacToken.balanceOf(address(raactReleaseOrchestrator)), 1985 ether);
raactReleaseOrchestrator.emergencyRevoke(beneficiary);
assertEq(raacToken.balanceOf(beneficiary), 985 ether);
assertEq(raacToken.balanceOf(address(raactReleaseOrchestrator)), 985 ether);
}
Run forge test --match-test test_wrongEmergencyAddressTransferMitigatio -vv
Logs:
raacToken: 0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f
raactReleaseOrchestrator 0x2e234DAe75C793f67A35089C9d99245E1C58470b
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 6.56ms (477.29µs CPU time)
All works fine.
