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

Inconsistent State Between Governance and TimelockController After Proposal Cancellation

mill1995

Summary

When a proposal is cancelled in the Governance contract after being queued, it remains active in the TimelockController. While this can be mitigated by the admin using the CANCELLER_ROLE to cancel the operation directly in the TimelockController, it creates an inconsistent state between the two contracts if this action is not called.

Vulnerability Details

The issue occurs in the cancel() function of the Governance contract. When a proposal is cancelled after being queued, the cancellation is only recorded in the Governance contract but not propagated to the TimelockController.

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");
}
// Only the internal proposal of the governance contract is set to canceled
@> proposal.canceled = true;
emit ProposalCanceled(proposalId, msg.sender, "Proposal canceled by proposer");
}

The TimelockControlleritself has a cancel() function that should be called during the cancelation process, to remove the scheduled operation from the Timelock contract:

function cancel(bytes32 id) external override onlyRole(CANCELLER_ROLE) {
if (_operations[id].timestamp == 0) {
revert OperationNotFound(id);
}
if (_operations[id].executed) {
revert OperationAlreadyScheduled(id);
}
​
delete _operations[id];
emit OperationCancelled(id);
}

PoC

In order to run the test you need to:

  1. Run foundryup to get the latest version of Foundry

  2. Install hardhat-foundry: npm install --save-dev @nomicfoundation/hardhat-foundry

  3. Import it in your Hardhat config: require("@nomicfoundation/hardhat-foundry");

  4. Make sure you've set the BASE_RPC_URL in the .env file or comment out the forking option in the hardhat config.

  5. Run npx hardhat init-foundry

  6. There is one file in the test folder that will throw an error during compilation so rename the file in test/unit/libraries/ReserveLibraryMock.sol to => ReserveLibraryMock.sol_broken so it doesn't get compiled anymore (we don't need it anyways).

  7. Create a new folder test/foundry

  8. Paste the below code into a new test file i.e.: FoundryTest.t.sol

  9. Run the test: forge test --mc FoundryTest -vvvv

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.19;
​
import {Test} from "forge-std/Test.sol";
import {Governance} from "../../contracts/core/governance/proposals/Governance.sol";
import {TimelockController} from "../../contracts/core/governance/proposals/TimelockController.sol";
import {RAACToken} from "../../contracts/core/tokens/RAACToken.sol";
import {veRAACToken} from "../../contracts/core/tokens/veRAACToken.sol";
import {IGovernance} from "../../contracts/interfaces/core/governance/proposals/IGovernance.sol";
import {ITimelockController} from "../../contracts/interfaces/core/governance/proposals/ITimelockController.sol";
import "forge-std/console2.sol";
​
contract FoundryTest is Test {
Governance public governance;
TimelockController public timelock;
RAACToken public raacToken;
veRAACToken public veToken;
​
address public admin = address(this);
address public proposer = makeAddr("proposer");
address public voter = makeAddr("voter");
​
// Proposal parameters
address[] public targets;
uint256[] public values;
bytes[] public calldatas;
string public description = "Test Proposal";
​
// Constants
uint256 public constant PROPOSAL_THRESHOLD = 100_000e18;
uint256 public constant VOTING_POWER = 1_000_000e18;
​
function setUp() public {
// Initial tax rates (1% swap tax, 0.5% burn tax)
uint256 initialSwapTaxRate = 100;
uint256 initialBurnTaxRate = 50;
​
raacToken = new RAACToken(admin, initialSwapTaxRate, initialBurnTaxRate);
​
raacToken.setMinter(admin);
​
veToken = new veRAACToken(address(raacToken));
​
// Setup timelock
address[] memory proposers = new address[](1);
proposers[0] = address(0);
address[] memory executors = new address[](1);
executors[0] = address(0);
timelock = new TimelockController(2 days, proposers, executors, admin);
​
// Deploy governance
governance = new Governance(address(veToken), address(timelock));
​
// Now setup correct timelock roles
timelock.grantRole(timelock.PROPOSER_ROLE(), address(governance));
timelock.grantRole(timelock.EXECUTOR_ROLE(), address(governance));
timelock.grantRole(timelock.EXECUTOR_ROLE(), admin);
// We also give the cancel role to the governance contract
timelock.grantRole(timelock.CANCELLER_ROLE(), address(governance));
​
// Setup proposal parameters
targets.push(address(raacToken));
values.push(0);
calldatas.push(abi.encodeWithSignature("approve(address,uint256)", address(1), 100));
}
​
function test_setupGovernanceTest() public {
assertEq(timelock.getRoleAdmin(timelock.PROPOSER_ROLE()), timelock.DEFAULT_ADMIN_ROLE());
assertEq(timelock.getRoleAdmin(timelock.EXECUTOR_ROLE()), timelock.DEFAULT_ADMIN_ROLE());
assertEq(timelock.getRoleAdmin(timelock.CANCELLER_ROLE()), timelock.DEFAULT_ADMIN_ROLE());
// Has role
assertEq(timelock.hasRole(timelock.PROPOSER_ROLE(), address(governance)), true);
assertEq(timelock.hasRole(timelock.EXECUTOR_ROLE(), address(governance)), true);
assertEq(timelock.hasRole(timelock.CANCELLER_ROLE(), address(governance)), true);
}
​
function test_CancelQueuedProposal() public {
// Mint voting power to proposer
raacToken.mint(proposer, VOTING_POWER);
​
// Create proposal
vm.startPrank(proposer);
raacToken.approve(address(veToken), VOTING_POWER);
veToken.lock(VOTING_POWER, 366 days);
​
uint256 proposalId = governance.propose(
targets,
values,
calldatas,
description,
IGovernance.ProposalType.ParameterChange
);
​
// Fast forward past voting delay
vm.warp(block.timestamp + governance.votingDelay() + 1);
​
// Cast vote
governance.castVote(proposalId, true);
​
// Fast forward past voting period
vm.warp(block.timestamp + governance.votingPeriod() + 1);
​
// Queue proposal (first execute call queues it)
governance.execute(proposalId);
​
// Cancel queued proposal
governance.cancel(proposalId);
vm.stopPrank();
​
// Verify proposal is canceled
assertEq(uint256(governance.state(proposalId)), uint256(IGovernance.ProposalState.Canceled));
​
bytes32 id = timelock.hashOperationBatch(targets, values, calldatas, bytes32(0), keccak256(bytes(description)));
// Now check in timelock contract if the proposal is canceled or still queued
assertEq(timelock.isOperationPending(id), true);
​
vm.warp(20 days);
​
// Now execute the proposal directly on the timelock still works
vm.expectEmit(true, true, true, true);
emit ITimelockController.OperationExecuted(
id,
targets,
values,
calldatas,
bytes32(0),
keccak256(bytes(description))
);
timelock.executeBatch(targets, values, calldatas, bytes32(0), keccak256(bytes(description)));
}
}
​

Impact

  • Creates inconsistent state between Governance and TimelockController

  • Requires additional admin intervention to maintain consistency

  • Could lead to unexpected execution of cancelled proposals if admin fails to cancel in TimelockController

Tools Used

  • Foundry

  • Manual Review

Recommendations

Modify the cancel() function in the Governance contract to also cancel the operation in the TimelockController when a queued proposal is cancelled and make sure the Governance contract has the CANCELLER_ROLE. This ensures that cancellation is atomic across both contracts and maintains consistent state:

function cancel(uint256 proposalId) external override {
//...
proposal.canceled = true;
// Cancel in timelock contract as well
if (currentState == ProposalState.Queued) {
bytes32 id = _timelock.hashOperationBatch(
proposal.targets,
proposal.values,
proposal.calldatas,
bytes32(0),
proposal.descriptionHash
);
_timelock.cancel(id);
}
}
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.