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

Funds are stuck in contract due to wrong calculation.

ke1caM

Summary

Calculations in _distributeRewards() use totalVotes to calculate how many pieces the reward should be divided to. This is incorrect way to do that because it divides the reward into more pieces, than it should be.

Vulnerability details

uint256 totalVotesFor = s_votersFor.length;
uint256 totalVotesAgainst = s_votersAgainst.length;
uint256 totalVotes = totalVotesFor + totalVotesAgainst;
​
...
​
uint256 totalRewards = address(this).balance;
​
...
​
uint256 rewardPerVoter = totalRewards / totalVotes;
​
for (uint256 i; i < totalVotesFor; ++i) {
// proposal creator is trusted when creating allowed list of voters,
// findings related to gas griefing attacks or sending eth
// to an address reverting thereby stopping the reward payouts are
// invalid. Yes pull is to be preferred to push but this
// has not been implemented in this simplified version to
// reduce complexity & help you focus on finding the
// harder to find bug
​
// if at the last voter round up to avoid leaving dust; this means that
// the last voter can get 1 wei more than the rest - this is not
// a valid finding, it is simply how we deal with imperfect division
if (i == totalVotesFor - 1) {
rewardPerVoter = Math.mulDiv(
totalRewards,
1,
totalVotes,
Math.Rounding.Ceil
);
}
_sendEth(s_votersFor[i], rewardPerVoter);
}

Here we calculate portion of the reward by dividing totalRewards by totalVotes instead of totalVotesFor .

Consider this situation:

There are 5 eligible voters.

First voter votes for (true), second voter votes against (false) and the third voter votes for (true).

This situation looks as follows:
Reward: 1ETH
The quorum has been reached.
Total votes: 3
Votes for: 2
Votes againt: 1

Reward for "for" voter is = reward / totalVotes. (1 ETH / 3) = 1/3 of the reward
When there are two for voters the reward is 2 * 1/3 = 2/3 which means that 1/3 of the reward is stuck in contract.

Due to the current design the reward will be divided into three pieces but only two will be sent meaning some portion will be stuck in contract because the function includes the against votes in calculations.

PoC

Please add this test to test/VotingBoothTest.t.sol and run forge test

function testDistributeToAll() public {
vm.prank(address(0x1));
booth.vote(true);
​
vm.prank(address(0x2));
booth.vote(false);
​
vm.prank(address(0x3));
booth.vote(true);
​
assert(!booth.isActive() && address(booth).balance > 0);
}

Impact

Funds are stuck in contract.

Tools used

VScode, Manual Review

Recommendations

To calcuate correct amount that has to be sent to "for" voter divide the reward by totalVotesFor not totalVotes to ensure corrent reward calculation and distribution.

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.