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

`VotingBooth::_distributeRewards` dilutes "for" voters rewards and locks unpaid amounts.

0xloscar01

Summary

The issue emerges after a proposal's approval during the voter reward calculation process. It stems from the formula used to calculate the reward per voter, which is the division of total rewards by the total number of votes. This total vote count is the sum of totalVotesFor and totalVotesAgainst.

https://github.com/Cyfrin/2023-12-Voting-Booth/blob/main/src/VotingBooth.sol#L172

uint256 totalVotes = totalVotesFor + totalVotesAgainst;

The total reward is divided by the sum of totalVotesFor and totalVotesAgainst, inadvertently including voters ineligible for rewards.

https://github.com/Cyfrin/2023-12-Voting-Booth/blob/main/src/VotingBooth.sol#L192

uint256 rewardPerVoter = totalRewards / totalVotes;

Consider a practical scenario: A proposal is funded with 10 ether and 7 voters are allowed to vote. It passes with 3 'for' votes and 1 'against' vote. According to the formula, the reward per voter is calculated as 2.5 ether (10 ether / 4 votes). This results in each 'for' voter receiving 2.5 ether, leaving 2.5 ether unclaimed. Due to the lack of a withdrawal mechanism in the VotingBooth contract, this remaining reward becomes inaccessible and is effectively locked within the contract.

Vulnerability Details

Steps to reproduce the test

  1. Import console and StdInvariant inside VotingBoothTest.t.sol:

import {Test, console} from "forge-std/Test.sol";
import {StdInvariant} from "forge-std/StdInvariant.sol";

  1. Copy and paste the following inside VotingBoothTest.t.sol::VotingBoothTest::setUp

for (uint256 i; i < voters.length; ++i) {
targetSender(voters[i]);
}
console.log("Voting has started");
console.log("VotingBooth balance: %s", address(booth).balance); // 10e18

  1. Copy and paste the function invariant_testAllMoneySentAfterProposalPassesOrReturnedAfterDefeated() inside VotingBoothTest.t.sol::VotingBoothTest

  2. Run forge test --mt invariant_testAllMoneySentAfterProposalPassesOrReturnedAfterDefeated -vv in the terminal.

Test case
// this test will fail because the invariant is not satisfied
function invariant_testAllMoneySentAfterProposalPassesOrReturnedAfterDefeated() public view {
// check if voting has ended
if (!booth.isActive()) {
console.log("Voting has ended");
console.log("Voting booth remaining balance: %s", address(booth).balance);
assert(address(booth).balance == 0); // check if the VotingBooth contract balance is 0 after voting has ended (rewards have been distributed or returned to the creator)
}
}
Test Logs
[FAIL. Reason: panic: assertion failed (0x01)]
[Sequence]
sender=0x0000000000000000000000000000000000000001 addr=[src/VotingBooth.sol:VotingBooth]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=vote(bool) args=[true]
sender=0x0000000000000000000000000000000000000004 addr=[src/VotingBooth.sol:VotingBooth]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=vote(bool) args=[false]
sender=0x0000000000000000000000000000000000000005 addr=[src/VotingBooth.sol:VotingBooth]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=vote(bool) args=[true]
invariant_testAllMoneySentAfterProposalPassesOrReturnedAfterDefeated() (runs: 256, calls: 3830, reverts: 3062)
Logs:
Voting has started
VotingBooth balance: 10000000000000000000
Voting has ended
Voting booth remaining balance: 3333333333333333333

Impact

'for' voters rewards are diluted and the residual rewards are locked in the VotingBooth contract.

Tools Used

Foundry

Recommendations

Consider calculating the reward per voter by dividing the total rewards by the total 'for' votes to avoid leaving unallocated funds locked in the 'VotingBooth' contract.

- uint256 rewardPerVoter = totalRewards / totalVotes;
+ uint256 rewardPerVoter = totalRewards/totalVotesFor;
​
- rewardPerVoter = Math.mulDiv(totalRewards, 1, totalVotes, Math.Rounding.Ceil);
+ rewardPerVoter = Math.mulDiv(totalRewards, 1, totalVotesFor, Math.Rounding.Ceil);

If the 'for' voters rewards dilution is intentional, implement a withdraw function that can be called by the owner after the proposal ends.

Updates

Lead Judging Commences

0xnevi Lead Judge almost 2 years 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.