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

Unsynchronized `cancel` functions in the `Governance` and `TimelockController` contracts allows execution of cancelled proposals

0xarav

Summary

Unsynchronized cancel functions in the Governance and the TimelockController
contracts allows any address with an EXECUTOR role to potentially execute cancelled proposals if they are already queued in the TimelockController

Vulnerability Details

State Inconsistency Between Governance and TimelockContoller:

The issue stems from lack of interaction between the `cancel` functions in the `Governance` contract and the `TimelockController`.

If a proposal is canceled after being queued in timelock,
the governance contract updates its state as proposal.canceled = true ,
but the queued operation remains executable in `TimelockController`.

This leaves the possibility that the timelock operation could still be executed,
bypassing Governance's cancel flag.

Step 1: Create a new proposal
`uint256 proposalId = governance.propose(...);`
// ... Votes are cast and proposal succeeds ...
​
Step 2: Queue the proposal
​
`governance.execute(proposalId);` // Internally calls _queueProposal()
// Timelock schedules the operation and maps it `_operations[id]`
​
Step 3: Proposer Cancels the proposal in Governance
​
vm.startPrank(proposer)
governance.cancel(proposalId);
​
// Governance sets
`proposal.canceled = true`,
//but does NOT call
`timelock.cancel(id)`
​
// Fast forward time past the delay so that the operation is executable
`vm.warp(block.timestamp + 3 days);`
​
Step 4: EXECUTOR directly execute the cancelled proposal in 'TimelockController'
​
vm.prank(EXECUTOR);
timelock.executeBatch();
​
Step 5: Proposal gets executed in `TimelockController`
even though it was cancelled by the proposer in 'Governance'

Issue: The state() function in governance returns Canceled if `proposal.canceled == true` ,
preventing normal execution via the governance execute() function.

However, since the timelock’s internal state isn’t updated.
This disconnect might allow an operation to exist in the timelock
that doesn’t reflect the governance decision.

The design of Role based access control,
indicates that there could be other addresses with EXECUTOR roles
other than just the governance contract address.

Allowing any address with the EXECUTOR role to execute the previously cancelled proposal.

Inversely when a Queued proposal is cancelled in the TimelockController
it is currently NOT reflected in the Governance contract's _proposals[id].cancelled

Impact

Cancelled proposals can be executed by any EXECUTOR
if the proposal's currentState == Queued during cancellation.

Impact : High
Likelihood : Med

Recommendations

Here are two suggested methods to resolve the issue

Method 1 : Synchronize Governance and Timelock Cancellation

Modify the cancellation process in the Governance contract
so that when a proposal is canceled,
the corresponding scheduled operation in the Timelock is also canceled.

ProposalState currentState = state(proposalId);
if (currentState == ProposalState.Queued) {
bytes32 operationId = _timelock.hashOperationBatch(
proposal.targets,
proposal.values,
proposal.calldatas,
bytes32(0),
proposal.descriptionHash
);
// This call requires the Governance contract to have the CANCELLER_ROLE in the Timelock.
_timelock.cancel(operationId);
}

Method 2 : Disallow cancellation if the proposal is already queued in the Timelock.

i.e, when `currentState == ProposalState.Queued`
This prevents any inconsistencies by ensuring that queued proposals cannot be canceled.

ProposalState currentState = state(proposalId);
​
if (currentState == ProposalState.Queued) {
revert InvalidProposalState(proposalId, currentState, ProposalState.Active, "Cannot cancel a queued proposal");
}

Furthermore, On both cases
when a Queued proposal is cancelled in the TimelockController
it needs to be reflected thru a state update of _proposals[id].cancelled in the Governanace contract .

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!