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

Stucked Reward

akhoronko

Summary

In case when there are votes "for" and "against" the reward is calculated incorrectly, as a result voters "for" receive less reward part than they should and part of the reward is stuck on the voting contract.

Vulnerability Details

The reward for a vote should be distributed among those who voted in favor, instead the payout is based on the total number of votes ("for" and "against"). Example: vote for 5 voters, payout is 1 ETH, 2 votes in favor and 1 against, quorum is reached and voting ends, those voters in favor expect to get 0.5 ETH each, but they will get 1/3 ETH each and rest 1/3 ETH will stuck on the contract forever.

Impact

Voters receive less reward that they should, part of reward stuck on the contract.

Tools Used

Visual Studio Code, Foundry test

Recommendations

Fix the _distributeRewards function as follows:

  1. replace line https://github.com/Cyfrin/2023-12-Voting-Booth/blob/a3241b1c530529a4f739f9462f720e8561ad45ca/src/VotingBooth.sol#L192

uint256 rewardPerVoter = totalRewards / totalVotes;

with

uint256 rewardPerVoter = totalRewards / totalVotesFor;

  1. replace line https://github.com/Cyfrin/2023-12-Voting-Booth/blob/a3241b1c530529a4f739f9462f720e8561ad45ca/src/VotingBooth.sol#L207

rewardPerVoter = Math.mulDiv(totalRewards, 1, totalVotes, Math.Rounding.Ceil);

with

rewardPerVoter = Math.mulDiv(totalRewards, 1, totalVotesFor, Math.Rounding.Ceil);

Working Test Case

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.23;
​
import {VotingBooth} from "../src/VotingBooth.sol";
import {Test} from "forge-std/Test.sol";
import {_CheatCodes} from "./mocks/CheatCodes.t.sol";
​
contract VotingBoothTest is Test {
// eth reward
uint256 constant ETH_REWARD = 10e18;
​
// allowed voters
address[] voters;
​
// contracts required for test
VotingBooth booth;
​
_CheatCodes cheatCodes = _CheatCodes(HEVM_ADDRESS);
​
function setUp() public virtual {
// deal this contract the proposal reward
deal(address(this), ETH_REWARD);
​
// setup the allowed list of voters
voters.push(address(0x1));
voters.push(address(0x2));
voters.push(address(0x3));
voters.push(address(0x4));
voters.push(address(0x5));
​
// setup contract to be tested
booth = new VotingBooth{value: ETH_REWARD}(voters);
​
// verify setup
//
// proposal has rewards
assert(address(booth).balance == ETH_REWARD);
// proposal is active
assert(booth.isActive());
// proposal has correct number of allowed voters
assert(booth.getTotalAllowedVoters() == voters.length);
// this contract is the creator
assert(booth.getCreator() == address(this));
}
​
// required to receive refund if proposal fails
receive() external payable {}
​
function testVotePassesAndMoneyIsSent() public {
// first voter - For
vm.prank(address(0x1));
booth.vote(true);
​
// second voter - For
vm.prank(address(0x2));
booth.vote(true);
​
// third voter - Against
vm.prank(address(0x3));
booth.vote(false);
​
// 3 / 5 votes, quorum is reached, 2 voters "For" should receive 0.5 ETH each
// and no ether should stuck on the contract
assert(address(booth).balance == 0); // will assert here
}
}
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.