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

Inconsistent Proposal Cancellation in Governance Contract Leads to Unexpected Execution of Cancelled Proposals

Fortis Audits

Summary:

The Governance::cancel function allows a proposal to be cancelled. However, if the proposal has already been queued in the TimelockController, calling Governance::cancel only updates the proposal's state within the Governance contract. It does not cancel the corresponding scheduled operation within the TimelockController. This creates an inconsistent state where the Governance contract reflects the proposal as cancelled, but the TimelockController still has the operation scheduled for execution.

Vulnerability Details:

The vulnerability arises from the lack of coordination between the Governance contract and the TimelockController during proposal cancellation. The Governance::cancel function focuses solely on updating the proposal's state within its own storage, specifically by setting the canceled flag in the _proposals mapping. It does not interact with the TimelockController to remove the corresponding operation from the timelock queue. This disconnect allows a proposal to be marked as cancelled in the Governance contract while the associated operation remains scheduled for execution in the TimelockController. This can lead to several undesirable outcomes, such as if a cancelled proposal's actions are still executed despite cancellation.

function cancel(uint256 proposalId) external override {
// ... other validations
​
ProposalState currentState = state(proposalId); // <-- Queued Proposal is not checked
if (currentState == ProposalState.Executed) {
revert InvalidProposalState(proposalId, currentState, ProposalState.Active, "Cannot cancel executed proposal");
}
​
// ... other checks ...
​
proposal.canceled = true;
emit ProposalCanceled(proposalId, msg.sender, "Proposal canceled by proposer");
}
​

Impact:

  • Inconsistent State: The state of the proposal is inconsistent between the Governance contract and the TimelockController. This can lead to confusion and make it difficult to determine the true status of a proposal..

  • Unexpected Execution: The system might behave unexpectedly if the cancelled proposal's actions are still executed. This could have significant consequences depending on the nature of the proposal.

Proof of Concept:

  1. Proposer create a proposal in the Governance contract. Ensure it gathers sufficient votes to pass quorum.

  2. Once the quorum is reached, call the Governance::execute function to queue the proposal in the TimelockController. This will schedule the operation for execution after the timelock delay.

  3. Later, proposer cancels the proposal by calling the Governance::cancel function.

  4. After cancelling, call the Governance::state function for the proposal. It will return ProposalState.Canceled.

  5. But,calling the TimelockController::isOperationPending function with the operation ID (obtainable by hashing the proposal details). It will return true, indicating the operation is still scheduled.

  6. As the cancelled proposal is still scheduled, the operation in the TimelockController will be still executable despite the Governance contract showing the proposal as cancelled, leading to unexpected execution.

Proof Of Code:

  1. Use this guide to intergrate foundry into your project: foundry

  2. Create a new file FortisAudits.t.sol in the test directory.

  3. Add the following gist code to the file: Gist Code

  4. Run the test using forge test --mt test_FortisAudits_InconsistentProposalCancellation -vvvv.

function test_FortisAudits_InconsistentProposalCancellation() public {
deal(anon, 100);
uint256 amount = 400_000e18;
vm.startPrank(initialOwner);
raacToken.setMinter(initialOwner);
raacToken.mint(anon, amount);
raacToken.mint(address(veraacToken), 100e18);
timelockController.grantRole(timelockController.EXECUTOR_ROLE(), initialOwner);
timelockController.grantRole(timelockController.PROPOSER_ROLE(), address(governance));
vm.stopPrank();
vm.startPrank(anon);
raacToken.approve(address(veraacToken), amount);
veraacToken.lock(amount, 365 days);
address[] memory targets = new address[](1);
targets[0] = address(treasury);
bytes[] memory data = new bytes[](1);
data[0] = abi.encodeWithSignature("allocateFunds(address,uint256)", anon, type(uint256).max);
uint256[] memory values = new uint256[](1);
values[0] = 10;
string memory description = "Proposal";
// Propose a treasury action for max allocation
uint256 proposalId = governance.propose(targets, values, data, "Proposal", IGovernance.ProposalType.TreasuryAction);
skip(1 days + 2);
governance.castVote(proposalId, true);
skip(7 days + 2);
// Proposal gets queued
governance.execute(proposalId);
// Proposer cancels the proposal
governance.cancel(proposalId);
(,,,,,,,,,bool cancelled) = governance.getDebugInfo(proposalId);
assertTrue(cancelled);
skip(7 days + 2);
bytes32 salt = bytes32(keccak256(bytes(description)));
// Time lock controller retains the proposal operations
bytes32 id = timelockController.hashOperationBatch(targets, values, data, bytes32(0), salt);
// Thus the operation is ready to be executed in time lock controller even though proposal is cancelled in governance
bool ready = timelockController.isOperationReady(id);
assertTrue(ready);
vm.stopPrank();
}

Recommended Mitigation:

The Governance::cancel function should call the TimelockController::cancel function to remove the corresponding operation from the timelock queue. This ensures that the proposal is consistently cancelled across both contracts and prevents unexpected execution of the proposal's actions.

// In Governance.sol
function cancel(uint256 proposalId) external override {
// ... other checks ...
​
ProposalState currentState = state(proposalId);
+ else if(currentState == ProposalState.Queued) {
+ bytes32 opertationId = _timelock.hashOperationBatch(
+ proposal.targets,
+ proposal.values,
+ proposal.calldatas,
+ bytes32(0), // Predecessor (if any)
+ proposal.descriptionHash // Salt
+ );
+ _timelock.cancel(operationId); // Call TimelockController's cancel function
+ }
​
proposal.canceled = true;
emit ProposalCanceled(proposalId, msg.sender, "Proposal canceled by proposer");
}
Updates

Lead Judging Commences

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

Governance::cancel and state lack synchronization with TimelockController

inallhonesty Lead Judge 7 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.

Give us feedback!