First Flight #6: Voting Booth
First Flight #6
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

`VotingBooth` 'for' voter rewards are diluted by 'against' voters and missing rewards permanently stuck in `VotingBooth` contract

EloiManuel

Summary

VotingBooth pays a reward to the for voters if the proposal passes, otherwise refunds the reward to the creator. In order for the proposal to pass, it needs to have more for votes than against. The system of voting doesn't require a strict majority to pass the proposal, it just requires the quorum to be reached (enough people to vote).

The problem arises when the for votes win, some voters voted against, and the rewards are calculated. Instead of equally dividing the reward pot between the for voters, it is dividing the reward between all users that votes (even though only the ones that voted for will end up receiving the reward). This wrong calculation of the rewards per voter could leave funds stuck in the contract.

Vulnerability Details

The rewardPerVoter calculation on line 192 of the VotingBooth.sol (https://github.com/Cyfrin/2023-12-Voting-Booth/blob/a3241b1c530529a4f739f9462f720e8561ad45ca/src/VotingBooth.sol#L192) is incorrect. The rewards are divided between the total number of votes and not the "for" voters.

Impact

If the voting booth has at least one against voter, the ETH reward paid out to for voters will be less than the total reward amount held by the VotingBooth contract and the missing balance will be permanently stuck inside the VotingBooth contract.

Tools Used

Foundry.

Proof of Concept

Add the following test function to the VotingBoothTest.t.sol file and run forge test -vvvv --mc VotingBoothTest --mt testExploit to see the exploit steps and result.

function testExploit() public {
assert(booth.isActive());
// User 1 votes against
vm.prank(address(0x1));
booth.vote(false);
​
// User 2 votes for
vm.prank(address(0x2));
booth.vote(true);
​
// User 3 votes for
vm.prank(address(0x3));
booth.vote(true);
​
assert(!booth.isActive());
​
// Voter 1 has 0 ETH
assert(address(0x1).balance == 0);
// Voter 1 has 1/3 of the reward
assert(address(0x2).balance == ETH_REWARD/3);
// Voter 2 has 1/3 of the reward
assert(address(0x3).balance == ETH_REWARD/3 + 1);
​
// The VotingBooth contract has 1/3 stuck
assert(address(booth).balance >= ETH_REWARD/3);
}

Recommendations

Consider changing the rewardPerVoter calculation on line 192 of the VotingBooth.sol (https://github.com/Cyfrin/2023-12-Voting-Booth/blob/a3241b1c530529a4f739f9462f720e8561ad45ca/src/VotingBooth.sol#L192) as shown below so the rewards are divided between the "for" voters and not the total number of votes:

else {
- uint256 rewardPerVoter = totalRewards / totalVotes;
+ uint256 rewardPerVoter = totalRewards / totalVotesFor;
​
for (uint256 i; i < totalVotesFor; ++i) {
Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

VotingBooth._distributeRewards(): Incorrect computation of rewardPerVoter

Support

FAQs

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