Description
In Governance, a proposal progresses through multiple stages. According to the documentation, the Succeeded stage occurs when a proposal has passed voting but has not yet been queued. Voting should only be allowed during the Active stage, which is determined by the following condition in the state function:
function state(uint256 proposalId) public view override returns (ProposalState) {
...
if (block.timestamp < proposal.endTime) return ProposalState.Active;
...
}
Additionally, the castVote function prevents users from voting only if block.timestamp > proposal.endTime. Since the state function treats block.timestamp == proposal.endTime as not Active, but castVote still allows voting at this timestamp, users can vote on an already Succeeded proposal, potentially altering the final outcome.
Context
Impact
Medium. Users can cast votes on a Succeeded proposal, which could change the final vote results in a way that was not intended.
Likelihood
Low. The only moment this can happen is precisely when block.timestamp == proposal.endTime.
Proof of Concept
pragma solidity ^0.8.19;
import {Test, console} from "../../lib/forge-std/src/Test.sol";
import {Governance, IGovernance} from "../../contracts/core/governance/proposals/Governance.sol";
import {TimelockController} from "../../contracts/core/governance/proposals/TimelockController.sol";
import {RAACToken} from "../../contracts/core/tokens/RAACToken.sol";
import {veRAACToken, IveRAACToken} from "../../contracts/core/tokens/veRAACToken.sol";
contract GovernanceTest is Test {
Governance governance;
TimelockController timelock;
RAACToken raac;
veRAACToken veRaac;
address OWNER = makeAddr("owner");
address PROPOSER = makeAddr("proposer");
address EXECUTOR = makeAddr("executor");
address USER = makeAddr("user");
function setUp() public {
vm.startPrank(OWNER);
address[] memory proposers = new address[](1);
proposers[0] = PROPOSER;
address[] memory executors = new address[](1);
executors[0] = EXECUTOR;
timelock = new TimelockController(2 days, proposers, executors, OWNER);
raac = new RAACToken(OWNER, 0, 0);
veRaac = new veRAACToken(address(raac));
governance = new Governance(address(veRaac), address(timelock));
timelock.grantRole(timelock.PROPOSER_ROLE(), address(governance));
timelock.grantRole(timelock.EXECUTOR_ROLE(), address(governance));
timelock.grantRole(timelock.CANCELLER_ROLE(), address(governance));
vm.stopPrank();
}
function test_wrongEndTimeCheck_castVote() public {
vm.mockCall(
address(veRaac),
abi.encodeWithSelector(bytes4(keccak256("getVotingPower(address)"))),
abi.encode(100_000e18)
);
address[] memory targets = new address[](1);
uint256[] memory values = new uint256[](1);
bytes[] memory calldatas = new bytes[](1);
string memory description = "Some Proposal";
IGovernance.ProposalType proposalType = IGovernance.ProposalType.ParameterChange;
vm.startPrank(PROPOSER);
uint256 proposalId = governance.propose(targets, values, calldatas, description, proposalType);
skip(1 days);
governance.castVote(proposalId, true);
vm.warp(governance.getProposal(proposalId).endTime);
assertEq(uint256(governance.state(proposalId)), uint256(IGovernance.ProposalState.Succeeded));
console.log(
"PROPOSAL STATE :",
governance.state(proposalId) == IGovernance.ProposalState.Succeeded ? "SUCCEEDED" : "NOT SUCCEEDED"
);
vm.startPrank(USER);
governance.castVote(proposalId, true);
(uint256 forVotes, uint256 againstVotes) = governance.getVotes(proposalId);
assertEq(forVotes + againstVotes, veRaac.getVotingPower(USER) + veRaac.getVotingPower(PROPOSER));
console.log("TOTAL VOTES :", forVotes + againstVotes);
}
}
Logs
Ran 1 test for test/foundry/GovernanceTest.t.sol:GovernanceTest
[PASS] test_wrongEndTimeCheck_castVote() (gas: 446242)
Logs:
PROPOSAL STATE : SUCCEEDED
TOTAL VOTES : 200000000000000000000000
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 17.85ms (3.29ms CPU time)
Instructions
First, integrate Foundry by running the following commands in your terminal, in the project's root directory:
mkdir out lib
git submodule add https://github.com/foundry-rs/forge-std lib/forge-std
touch foundry.toml
Next, configure Foundry by adding the following settings to foundry.toml:
[profile.default]
src = "contracts"
out = "out"
lib = "lib"
After that, create a foundry/ directory inside the test/ directory. Inside foundry/, create the following files:
Finally, paste the provided (PoC) into GovernanceTest.t.sol and run:
forge test --mt test_wrongEndTimeCheck_castVote -vvv
Recommendation
Consider changing the proposal.endTime check in the castVote function:
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) {
+ 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;
}
