Submission Details
Severity:
A user can call execute before the end of Vote casting
Summary
A user can call execute before the end of vote casting and execute the proposal based to the votes available. This can be weaponized on ETH where front running is possible, if the last vote is enough to over turn a decision, a user can just execute a proposal immediately.
Vulnerability Details
Vote casting is between start till end
/**
* @notice Casts a vote on a governance proposal
* @dev Allows veToken holders to vote on active proposals
* - One vote per proposal per address
* - Vote weight based on veToken voting power
* - Supports yes/no voting
* - Voting window between start and end times
* @param proposalId The ID of the proposal to vote on
* @param support True to vote in favor, false to vote against
* @return weight The voting power used for this vote
*/
function castVote(uint256 proposalId, bool support) external override returns (uint256) { // if proposal has reached limit stop it else attack can arise
ProposalCore storage proposal = _proposals[proposalId];
if (proposal.startTime == 0) revert ProposalDoesNotExist(proposalId);
@audit>> if (block.timestamp < proposal.startTime) {
revert VotingNotStarted(proposalId, proposal.startTime, block.timestamp);
}
@audit>> if (block.timestamp > proposal.endTime) {
revert VotingEnded(proposalId, proposal.endTime, block.timestamp); // vote can be casted at block.timestamp == end time
}
ProposalVote storage proposalVote = _proposalVotes[proposalId];
if (proposalVote.hasVoted[msg.sender]) {
revert AlreadyVoted(proposalId, msg.sender, block.timestamp);
}
uint256 weight = _veToken.getVotingPower(msg.sender);
if (weight == 0) {
revert NoVotingPower(msg.sender, block.number);
}
proposalVote.hasVoted[msg.sender] = true;
if (support) {
proposalVote.forVotes += weight;
} else {
proposalVote.againstVotes += weight;
}
emit VoteCast(msg.sender, proposalId, support, weight, "");
return weight;
}
BUT ALSO EXECUTION CAN BE CALLED 1 second to the end of vote casting, this overlap creates an avenue for decisions to be made without vote casting completion.
* @dev Two-step execution process:
* 1. Queue - Schedule proposal in timelock when vote succeeds
* 2. Execute - Execute proposal after timelock delay
* @param proposalId The ID of the proposal to execute
*/
function execute(uint256 proposalId) external override nonReentrant {
ProposalCore storage proposal = _proposals[proposalId];
@audit>>> @audit>>> if (proposal.executed) revert ProposalAlreadyExecuted(proposalId, block.timestamp);
@audit>>> ProposalState currentState = state(proposalId);
// Check if the proposal is in the correct state for execution
if (currentState == ProposalState.Succeeded) {
// Queue the proposal
_queueProposal(proposalId);
@audit>>> } else if (currentState == ProposalState.Queued) {
// Execute the queued proposal
@audit>>> _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"
);
}
}
*/
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;
if (block.timestamp < proposal.startTime) return ProposalState.Pending;
@audit>>> if (block.timestamp < proposal.endTime) return ProposalState.Active; <= note // last 1 second vote can change it all
// After voting period ends, check quorum and votes
ProposalVote storage proposalVote = _proposalVotes[proposalId];
@audit>> uint256 currentQuorum = proposalVote.forVotes + proposalVote.againstVotes;
@audit>> uint256 requiredQuorum = quorum();
// Check if quorum is met and votes are in favor
@audit>> if (currentQuorum < requiredQuorum || proposalVote.forVotes <= proposalVote.againstVotes) { // according to comment above and not or
return ProposalState.Defeated;
}
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;
}
// If not pending and voting passed, it's Succeeded
return ProposalState.Succeeded;
}
At if block.timestamp is equal to end time the state can process the votes but also the vote casting is not over because a vote casted at this time will also be recorded since block.timestamp is not greater than end time
Impact
Execution can be called before the last batch of votes are casted.
Tools Used
Manual Review
Recommendations
change the check in the state function to prevent an overlapping between this functions
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;
if (block.timestamp < proposal.startTime) return ProposalState.Pending;
-- if (block.timestamp < proposal.endTime) return ProposalState.Active; <= note // last 1 second vote can change it all
++ if (block.timestamp <= proposal.endTime) return ProposalState.Active;
Updates
Lead Judging Commences
inallhonesty 3 months ago
Submission Judgement Published Assigned finding tags:
Governance::state and Governance::castVote use inconsistent time boundary checks, allowing votes at exactly proposal.endTime when state shows inactive
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.