Core Contracts

Summary

The cancel function in the Governance contract does not call the corresponding cancel function in the TimeLock contract. This omission creates an inconsistent state where a canceled proposal remains scheduled in the TimeLock contract and can still be executed by an admin, leading to potential unintended consequences.

Vulnerability Details

1. Governance Contract:

   • The cancel function marks the proposal as canceled but does not call the cancel function in the TimeLock contract.

   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; // audit cancel function is not called

   emit ProposalCanceled(proposalId, msg.sender, "Proposal canceled by proposer");

   }

2. TimeLock Contract:

   • The cancel function in the TimeLock contract allows an admin to cancel a scheduled operation.

   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);

   }

Root Cause

When a proposal is canceled in the Governance contract, it only marks the proposal as canceled locally (proposal.canceled = true) but does not propagate this cancellation to the TimeLock contract. As a result, the proposal remains scheduled in the TimeLock contract and can still be executed by an admin, leading to an inconsistent state between the two contracts.

Impact

Inconsistent State:

• A canceled proposal remains scheduled in the TimeLock contract, creating a mismatch between the Governance and TimeLock states.

Unintended Execution:

• An admin with the EXECUTOR_ROLE in the TimeLock contract can still execute the canceled proposal, leading to unintended and potentially harmful actions.

Tools Used

Manual Code Review

Recommendations

Update the cancel function in the Governance contract to call the cancel function in the TimeLock contract. This ensures that the cancellation is propagated to the TimeLock contract, maintaining consistency between the two contracts.