Summary
voting is complete prematurely which stop other eligible users from voting which can decide the result of proposal.
Vulnerability Details
In vote function user can cast vote on proposal. It checks if voting is active or not, then allow eligible users to vote. function is given below.
function vote(bool voteInput) external {
require(isActive(), "DP: voting has been completed on this proposal");
address voter = msg.sender;
require(s_voters[voter] == ALLOWED, "DP: voter not allowed or already voted");
s_voters[voter] = VOTED;
uint256 totalCurrentVotes = ++s_totalCurrentVotes;
if (voteInput) s_votersFor.push(voter);
else s_votersAgainst.push(voter);
@> if (totalCurrentVotes * 100 / s_totalAllowedVoters >= MIN_QUORUM) {
@>
@> s_votingComplete = true;
_distributeRewards();
}
}
problem arises within the if block implemented to check MIN_QUOROM after every vote. Affected code is highlighted. This create a specific scenerio, where voting is stopped prematurely, even there is user available so vote which can reverse the outcome.
Consider a scenario, where 5 users are allowed by the owner to vote. Now 1st two users vote cast their vote as true(for), Now 3rd voter cast his vote as false (against) the proposal. As per current logic, totalvotes (3) are more than 51% of total allowed (5) votes. So s_votingComplete will become true, which prevents the other eligible voters from voting.
Due to this flawed logic, actual outcome is affected, that could have been resulted exactly opposite, if all allowed users could vote.
POC
In current test suite, update import and setUp function like below:
- import {Test} from "forge-std/Test.sol";
+ import {Test, console2} from "forge-std/Test.sol";
then add this function -
function testAllUsersCantVote() public {
vm.prank(address(0x1));
booth.vote(true);
vm.prank(address(0x2));
booth.vote(true);
vm.prank(address(0x3));
booth.vote(false);
console2.log("voting is closed prematurely:", !booth.isActive());
vm.prank(address(0x4));
vm.expectRevert();
booth.vote(false);
}
Run forge test --mt testAllUsersCantVote -vvv in your terminal, you'll get following results
[⠆] Compiling...
[⠒] Compiling 1 files with 0.8.23
[⠑] Solc 0.8.23 finished in 1.33s
Compiler run successful!
Running 1 test for test/VotingBoothTest.t.sol:VotingBoothTest
[PASS] testAllUsersCantVote() (gas: 260180)
Logs:
voting is closed prematurely: true
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 5.38ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)
Impact
Defeat the whole purpose of voting.
Tools Used
Manual Review, Foundry
Recommendations
Allow all eligible voters to vote to correctly being able to get fair voting on purposal
Here is the recommendation --
// record a vote
function vote(bool voteInput) external {
// prevent voting if already completed
require(isActive(), "DP: voting has been completed on this proposal");
// current voter
address voter = msg.sender;
// prevent voting if not allowed or already voted
require(s_voters[voter] == ALLOWED, "DP: voter not allowed or already voted");
// update storage to record that this user has voted
s_voters[voter] = VOTED;
// update storage to increment total current votes
// and store new value on the stack
uint256 totalCurrentVotes = ++s_totalCurrentVotes;
// add user to either the `for` or `against` list
if (voteInput) s_votersFor.push(voter);
else s_votersAgainst.push(voter);
// check if quorum has been reached. Quorum is reached
// when at least 51% of the total allowed voters have cast
// their vote. For example if there are 5 allowed voters:
//
// first votes For
// second votes For
// third votes Against
//
// Quorum has now been reached (3/5) and the vote will pass as
// votesFor (2) > votesAgainst (1).
//
// This system of voting doesn't require a strict majority to
// pass the proposal (it didn't require 3 For votes), it just
// requires the quorum to be reached (enough people to vote)
//
- if (totalCurrentVotes * 100 / s_totalAllowedVoters >= MIN_QUORUM) {
+ uint256 votesFor = s_votersFor.length;
+ uint256 voesAgainst = s_votersAgainst.length;
+ if (totalCurrentVotes * 100 / s_totalAllowedVoters >= MIN_QUORUM && totalCurrentVotes == s_totalAllowedVoters) {
// mark voting as having been completed
s_votingComplete = true;
// distribute the voting rewards
_distributeRewards();
}
}
