Summary
RAACReleaseOrchestrator::createVestingSchedule creates vesting schedules for distributing RAAC tokens to various stakeholders, but fails to transfer the actual tokens (RAACToken) during schedule creation. When beneficiary later attempts to claim their vested tokens via the RAACReleaseOrchestrator::release function, the transfers will fail if the contract doesn't hold the necessary tokens.
Vulnerability Details
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();
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.initialized = true;
emit VestingScheduleCreated(beneficiary, category, amount, startTime);
}
Impact
The function records the vesting commitment but fails to transfer the RAACToken to the contract. The RAACReleaseOrchestrator::createVestingSchedule will create unbackable vesting commitments. Subsequently, all RAACReleaseOrchestrator::release calls will revert due to insufficient balance. The vesting process doesn't work as expected.
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_createVestingDoesntTransferToken() 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), 1000 ether);
assertEq(raacToken.balanceOf(address(raactReleaseOrchestrator)), 1000 ether);
}
}
Run forge test --match-test test_createVestingDoesntTransferToken -vv
Logs:
raacToken: 0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f
raactReleaseOrchestrator 0x2e234DAe75C793f67A35089C9d99245E1C58470b
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 6.64ms (511.67µs CPU time)
The test shows that no RAACToken are transferred from beneficiary to the contract.
Tools Used
Manual review
Recommendations
Add the safeTransferFrom in the RAACReleaseOrchestrator::createVestingSchedule function.
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;
+ raacToken.safeTransferFrom(beneficiary, address(this), amount);
VestingSchedule storage schedule = vestingSchedules[beneficiary];
schedule.totalAmount = amount;
schedule.startTime = startTime;
schedule.duration = VESTING_DURATION;
schedule.initialized = true;
emit VestingScheduleCreated(beneficiary, category, amount, startTime);
}
Now copy this test
function test_createVestingDoesntTransferTokenMitigation() 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);
}
and run forge test --match-test test_createVestingDoesntTransferTokenMitigation -vv
Logs:
raacToken: 0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f
raactReleaseOrchestrator 0x2e234DAe75C793f67A35089C9d99245E1C58470b
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 6.96ms (349.75µs CPU time)
All works fine.
