sherlock-audit / 2024-01-olympus-on-chain-governance-judging

9 stars 7 forks source link

cawfree - Griefing: Proposers with marginal voting power in excess of `getProposalThresholdVotes()` can have their proposals terminated immediately by an adversarial delegator. #47

Closed sherlock-admin closed 9 months ago

sherlock-admin commented 9 months ago

cawfree

medium

Griefing: Proposers with marginal voting power in excess of getProposalThresholdVotes() can have their proposals terminated immediately by an adversarial delegator.

Summary

Proposers must maintain a proposal threshold exactly above getProposalThresholdVotes() in order to prevent their proposal from being cancelled.

However, an adversary may merely deallocate small units of pre-existing delegation power to unfairly cancel an ProposalStatus.Active proposal.

Vulnerability Detail

A proposer must possess at a minimum greater than getProposalThresholdVotes() to submit a proposal:

// Allow addresses above proposal threshold and whitelisted addresses to propose
if (
    gohm.getPriorVotes(msg.sender, block.number - 1) <= getProposalThresholdVotes() &&
    !isWhitelisted(msg.sender)
) revert GovernorBravo_Proposal_ThresholdNotMet();

If a proposal has not been executed, it may be cancelled via a call to cancel(uint256) by any caller, on the condition the proposer's voting power has fallen below or equal to the getProposalThresholdVotes():

if (
    gohm.getPriorVotes(proposal.proposer, block.number - 1) >=
    proposal.proposalThreshold
) revert GovernorBravo_Cancel_AboveThreshold();

This is regardless of how low their voting power has fallen, even if by a single vote, enabling adversaries to a marginal proposer to take an interested stake and use this to their advantage.

In this instance, we'll use the term "marginal proposer" to mean a member of the governance process who barely meets the governance threshold for submitting proposals.

The resulting affect is that an adversary can take a small amount of voting power away from a proposer, cancel their proposal, then re-instate delegation. In the context of governance, this could be extremely frustrating for an enthusiastic new member, undue confusion about the process leading to diminished opinion of the proposer by the voting audience on the social consensus layer, and bullying.

Impact

Griefing of marginal proposers to the governance process, and malicious gatekeeping of the ability to affect meaningful change in the protocol, particularly to new entrants.

Code Snippet

/**
 * @notice Cancels a proposal only if sender is the proposer, or proposer delegates dropped below proposal threshold
 * @param proposalId The id of the proposal to cancel
 */
function cancel(uint256 proposalId) external {
    if (state(proposalId) == ProposalState.Executed)
        revert GovernorBravo_Cancel_AlreadyExecuted();

    Proposal storage proposal = proposals[proposalId];

    // Proposer can cancel
    if (msg.sender != proposal.proposer) {
        // Whitelisted proposers can't be canceled for falling below proposal threshold
        if (isWhitelisted(proposal.proposer)) {
            if (
                (gohm.getPriorVotes(proposal.proposer, block.number - 1) >= proposal.proposalThreshold) ||
                msg.sender != whitelistGuardian
            ) revert GovernorBravo_Cancel_WhitelistedProposer();
        } else {
            if (gohm.getPriorVotes(proposal.proposer, block.number - 1) >= proposal.proposalThreshold)
                revert GovernorBravo_Cancel_AboveThreshold();
        }
    }

    proposal.canceled = true;
    for (uint256 i = 0; i < proposal.targets.length; i++) {
        timelock.cancelTransaction(
            proposal.targets[i],
            proposal.values[i],
            proposal.signatures[i],
            proposal.calldatas[i],
            proposal.eta
        );
    }

    emit ProposalCanceled(proposalId);
}

Tool used

Vim, Foundry

Recommendation

It is advised that callers should only have the ability to cancel(uint256) if the proposer's voting weight has fallen a meaningful amount, i.e. 25%.

This will increase the opportunity cost for a single attacker to take an interested stake in a competitor for an extensive period of time, in addition to increasing the overall amount of collusion required to successfully execute the attack.

nevillehuang commented 9 months ago

Invalid, intended functionality, if proposer cannot maintain his voting power, the proposal should be cancelled as indicated by comemnts here