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

cancelled proposals can still be executed through the timelock controller in the Governance contract

udogodwin2k22

Summary

The Governance contract allows queued proposals to be executed through the timelock even after being cancelled. This occurs because the cancellation mechanism fails to invalidate the corresponding timelock operation, creating a disconnect between the proposal's cancelled state and its excitability. The cancel function sets a proposal’s canceled flag to true but doesn’t interact with the timelock to cancel queued operations, allowing them to remain executable.

Vulnerability Details

When a proposal is cancelled via the cancel() function, it only sets a local flag without affecting the already queued timelock operation. A proposal queued in the timelock before cancellation can still be executed via _executeProposal because state returns Queued (based on timelock status) until executed, overriding the canceled flag.

​
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");
​
}
​
proposal.canceled = true;
​
emit ProposalCanceled(proposalId, msg.sender, "Proposal canceled by proposer");
​
}
​

The problem is worsened by the state() function's logic, which determines a proposal's status primarily based on the timelock's state rather than the proposal's cancelled flag

​
function state(uint256 proposalId) public view override returns (ProposalState) {
​
ProposalCore storage proposal = _proposals[proposalId];
​
if (proposal.startTime == 0) revert ProposalDoesNotExist(proposalId);
​
if (proposal.canceled) return ProposalState.Canceled;
​
if (proposal.executed) return ProposalState.Executed;
​
​
// ... voting period checks ...
​
bytes32 id = _timelock.hashOperationBatch(
​
proposal.targets,
​
proposal.values,
​
proposal.calldatas,
​
bytes32(0),
​
proposal.descriptionHash
​
);
​
// If operation is pending in timelock, it's Queued
​
if (_timelock.isOperationPending(id)) {
​
return ProposalState.Queued;
​
}
​
}
​

This creates a situation where a cancelled proposal can still be executed through _executeProposal() because the timelock operation remains valid and executable.

function execute(uint256 proposalId) external override nonReentrant {
ProposalCore storage proposal = _proposals[proposalId];
if (proposal.executed) revert ProposalAlreadyExecuted(proposalId, block.timestamp);
ProposalState currentState = state(proposalId);
// Check if the proposal is in the correct state for execution
if (currentState == ProposalState.Succeeded) {
// Queue the proposal
_queueProposal(proposalId);
} else if (currentState == ProposalState.Queued) {
// Execute the queued proposal
_executeProposal(proposalId);
} else {
// If not in Succeeded or Queued state, revert
revert InvalidProposalState(
proposalId,
currentState,
currentState == ProposalState.Active ? ProposalState.Succeeded : ProposalState.Queued,
"Invalid state for execution"
);
}
}

When a proposal is cancelled after being queued in the timelock, the queued operation remains valid and executable through the TimelockController's executeBatch() function

​
function executeBatch(
​
address[] calldata targets,
​
uint256[] calldata values,
​
bytes[] calldata calldatas,
​
bytes32 predecessor,
​
bytes32 salt
​
) external override payable nonReentrant onlyRole(EXECUTOR_ROLE)
​

Impact

  1. Cancelled proposals can still be executed using the timelock, making cancellations ineffective.

  2. This also makes the time-weighted voting system unreliable, since cancelled proposals can still impact the protocol.

Tools Used

Manual code review

Recommendations

Modify the cancel() function to properly invalidate timelock operations:

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");
}
​
// Cancel timelock operation if already queued
if (currentState == ProposalState.Queued) {
bytes32 id = _timelock.hashOperationBatch(
proposal.targets,
proposal.values,
proposal.calldatas,
bytes32(0),
proposal.descriptionHash
);
_timelock.cancel(id);
}
​
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!