Skip to content

Future Proposal Cancellation#

Low Risk

Vulnerability#

Any user is able to call cancel on a future proposal, causing a ProposalCanceled event to be emitted and the canceled boolean to be set to true for a given proposal before it is even created.

It is important to note that the canceled variable will be overridden to false when the proposal is actually created.

This works because before a proposal is created, the "proposer" address is the zero address (unset state). The zero address has no delegate votes and thus will be below the proposal threshold, so anyone can cancel the proposal.

Note that this also works for the invalid zero id proposal, although this is less exploitable.

Test Case#

it('can cancel a future proposal (undesirable behavior)', async function () {
  await expect(governor.cancel(100))
    .to.emit(governor, 'ProposalCanceled')
    .withArgs(100);
});

Exploit#

This bug could be used to confuse government UIs which query cancellation events to display active proposals. While the governance UI implementation is out of the scope of this audit, it is reasonable to believe a UI might hide proposals that had emitted cancellation events. A malicious proposer might use this to pre-cancel their proposal and obfuscate it from governance participants.

As a result, a proposal might receive less participation or review and malicious proposals could be passed that otherwise would have been promptly defeated.

Recommendation#

There are two state functions in the governance contract, a high level one that accepts proposalId and a lower level helper which accepts proposal and proposalVote structs.

Throughout the contract, calls are made directly to the lower level function which does not validate proposalId, allowing for bugs like cancelling future proposals.

We recommend:

  1. Moving proposalCount >= proposalId validation into the lower level function
  2. Additionally validating proposalId != 0

This will cause cancel to revert with an invalid proposalId as it calls directly to the lower level state helper.

/**
  * @notice Gets the state of a proposal
  * @param proposalId The id of the proposal
  * @return Proposal state
  */
function state(uint256 proposalId) public view returns (ProposalState) {
    Proposal storage proposal = proposals[proposalId];
    ProposalVote storage proposalVote = proposalVotes[proposalId];

    return state(proposal, proposalVote, proposalId);
}

/**
  * @notice Private function that gets the state of a proposal.
  * @param proposal The proposal struct
  * @param proposalVote The proposal vote struct
  * @return Proposal state
  */
function state(Proposal storage proposal, ProposalVote storage proposalVote, uint256 proposalId)
    private
    view
    returns (ProposalState)
{
    require(
        proposalCount >= proposalId,
        "Governor::state: invalid proposal id"
    );
    if (proposal.canceled) {
        return ProposalState.Canceled;
    } else if (block.timestamp <= proposal.startTime) {
        return ProposalState.Pending;
    } else if (block.timestamp <= proposal.endTime) {
        return ProposalState.Active;
    } else if (
        proposalVote.forVotes <= proposalVote.againstVotes ||
        (proposalVote.forVotes +
            proposalVote.againstVotes +
            proposalVote.abstainVotes) <
        proposal.quorum
    ) {
        return ProposalState.Defeated;
    } else if (proposal.executed) {
        return ProposalState.Executed;
    } else if (
        block.timestamp >=
        proposal.endTime + TIMELOCK_DELAY + TIMELOCK_GRACE_PERIOD
    ) {
        return ProposalState.Expired;
    } else if (block.timestamp < proposal.endTime + TIMELOCK_DELAY) {
        return ProposalState.Queued;
    } else {
        return ProposalState.Executable;
    }
}