Core Contracts
Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

Inconsistent time boundary check in `Governance::state` and `Governanane::castVote`

kiteweb3

Summary

The Governance contract implements two functions: state(which determines a proposal's current state, and castVote which allows users to vote on active proposals. These functions use different boundary conditions when checking the proposal's end time. The inconsistent use of boundary creates a time window where votes cast at exactly proposal.endTime will be accepted and counted. However, these votes are cast on a proposal that state considers inactive.

Vulnerability Details

function castVote(uint256 proposalId, bool support) external override returns (uint256) {
ProposalCore storage proposal = _proposals[proposalId];
if (proposal.startTime == 0) revert ProposalDoesNotExist(proposalId);
if (block.timestamp < proposal.startTime) {
revert VotingNotStarted(proposalId, proposal.startTime, block.timestamp);
}
@> if (block.timestamp > proposal.endTime) {
revert VotingEnded(proposalId, proposal.endTime, block.timestamp);
}
​
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;
}
​
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;
​
// After voting period ends, check quorum and votes
ProposalVote storage proposalVote = _proposalVotes[proposalId];
uint256 currentQuorum = proposalVote.forVotes + proposalVote.againstVotes;
uint256 requiredQuorum = quorum();
​
// Check if quorum is met and votes are in favor
if (currentQuorum < requiredQuorum || proposalVote.forVotes <= proposalVote.againstVotes) {
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;
}
​

Impact

Add Foundry to the project following this procedure

Create a file named Governance.t.sol and copy/paste this:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.0;
​
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_canVoteOnEndedProposal() public {
// Set up voting power for the owner
veToken.mock_setInitialVotingPower(owner, 4000000 ether);
//owner creates 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();
​
vm.warp(block.timestamp + governance.votingDelay() + governance.votingPeriod());
//The state returns an inactive proposal: status is defeated
IGovernance.ProposalState checkSucceedProposalState = governance.state(0);
assertEq(uint256(checkSucceedProposalState), 3); //defeated
(uint256 forBefore, ) = governance.getVotes(0);
//Owner can cast a vote
vm.startPrank(owner);
governance.castVote(0, true);
vm.stopPrank();
​
(uint256 forAfter, ) = governance.getVotes(0);
​
(, uint256 startTime, uint256 endTime, uint256 currentTime, , , , , , ) = governance.getDebugInfo(0);
​
assertEq(currentTime, endTime);
assertEq(forAfter, 4000000 ether);
assertEq(forAfter, forBefore + 4000000 ether);
​
console2.log("endTime: ", endTime);
console2.log("currentTime: ", currentTime);
}
}

Run forge test --match-test test_canVoteOnEndedProposal -vv

[PASS] test_canVoteOnEndedProposal() (gas: 775808)
Logs:
endTime: 691201
currentTime: 691201
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 10.06ms (1.53ms CPU time)

The test shows that at block.timestamp == proposal.endTime, the state returns an inactive proposal (status = defeated) but the owner can cast a vote.

At exactly block.timestamp == proposal.endTime:

  • state considers the proposal inactive

  • castVote still allows voting

This creates a logical contradiction in the contract's state. Users relying on state might incorrectly assume voting has ended. Malicious users could intentionally target the endTime block to cast votes when other users believe voting has ended leading to last-second vote manipulation that's difficult for other participants to counter.

Tools Used

Manual review

Recommendations

Align the time boundary checks in both 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;
+ if (block.timestamp <= proposal.endTime) return ProposalState.Active;
​
// After voting period ends, check quorum and votes
ProposalVote storage proposalVote = _proposalVotes[proposalId];
uint256 currentQuorum = proposalVote.forVotes + proposalVote.againstVotes;
uint256 requiredQuorum = quorum();
​
// Check if quorum is met and votes are in favor
if (currentQuorum < requiredQuorum || proposalVote.forVotes <= proposalVote.againstVotes) {
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;
}
​
Updates

Lead Judging Commences

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Governance::state and Governance::castVote use inconsistent time boundary checks, allowing votes at exactly proposal.endTime when state shows inactive

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.