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

Rewards Can Get Stuck in the Voting Booth

pacelli

Summary

Calculation of rewards after a proposal has passed may result in funds getting permanently stuck in the voting booth.

Vulnerability Details

After a proposal has passed the contract calculates how much reward each For voter has earned and runs a loop to reward those voters.

uint256 rewardPerVoter = totalRewards / totalVotes;
​
for (uint256 i; i < totalVotesFor; ++i) {
if (i == totalVotesFor - 1) {
rewardPerVoter = Math.mulDiv(totalRewards, 1, totalVotes, Math.Rounding.Ceil);
}
_sendEth(s_votersFor[i], rewardPerVoter);
}

The problem with how rewardPerVoter is calculated is that totalVotes is assumed to represent all the For voters when in fact it accounts for all the cast votes (For and Against). In a situation in which a passed proposal contains a mix of votes, rewardPerVoter is calculated but only the For voters are rewarded and this results in an amount of rewards getting stuck in the contract.

Let's assume 7 voters were allowed to cast their vote in a proposal and the reward to distribute is 1 ETH. Quorum is reached (55%) after the first 4 voters cast their votes in the following way: true, true, true, false, this means there are 3 For voters and 1 Against voter. The reward per voter is calculated as such:

rewardPerVoter = totalRewards / totalVotes
rewardPerVoter = 1 ETH / 4 voters
rewardPerVoters = 0.25 ETH / voter

Then the loop runs to reward the For voters and this will result in 0.25 ETH getting stuck in the contract forever, because 1 of the voters voted against the proposal but was taken into consideration to calculate the reward.

Proof Of Concept

We can leverage Foundry's fuzzing tests capabilities to extend the example from above and prove that there is a wide range of votes combinations that will result in the same outcome.

Copy and paste the following test case in VotingBoothTest.t.sol to see the effect of this bug.

POC
function testFuzzSecurityReview__BoothBalanceMustBeZeroAfterVoteConclude(uint256 numVoters, bool[] memory votes)
public
{
// bound num of voters between 3 and 9
numVoters = bound(numVoters, 3, 9);
// restrict num of voters to be an odd number
vm.assume(numVoters % 2 != 0);
// set length of votes array to be 9 to always have enough votes to cast
vm.assume(votes.length == 9);
​
// initialize a creator
address creator = makeAddr("creator");
// deal eth to the creator
deal(creator, ETH_REWARD);
​
// initialize array in memory to hold the voters
address[] memory allowedVoters = new address[](numVoters);
​
// populate array, start at index 1 because address(0) it's forbidden
for (uint256 i; i < numVoters; i++) {
allowedVoters[i] = address(uint160(i + 1));
}
​
// deploy booth
vm.prank(creator, creator);
VotingBooth votingBooth = new VotingBooth{value: ETH_REWARD}(allowedVoters);
​
// while vote is active, prank a voter and cast vote
for (uint256 i; i < numVoters; i++) {
if (votingBooth.isActive()) {
// assert while vote is active the booth must have the original balance
assertEq(address(votingBooth).balance, ETH_REWARD);
vm.prank(allowedVoters[i], allowedVoters[i]);
votingBooth.vote(votes[i]);
}
}
​
// assert booth's balance is zero after voting concludes
if(!votingBooth.isActive()) assertEq(address(votingBooth).balance, 0);
}

Impact

Rewards permanently stuck in the contract.

Tools Used

Manual verification and Foundry's fuzz test.

Recommendations

Update VotingBooth::_distributRewards to only use the For voters to calculate the distribute the rewads.

function _distributeRewards() private {
uint256 totalVotesFor = s_votersFor.length;
uint256 totalVotesAgainst = s_votersAgainst.length;
- uint256 totalVotes = totalVotesFor + totalVotesAgainst;
​
uint256 totalRewards = address(this).balance;
​
if (totalVotesAgainst >= totalVotesFor) {
_sendEth(s_creator, totalRewards);
}
else {
- uint256 rewardPerVoter = totalRewards / totalVotes;
+ uint256 rewardPerVoter = totalRewards / totalVotesFor;
​
for (uint256 i; i < totalVotesFor; ++i) {
if (i == totalVotesFor - 1) {
- rewardPerVoter = Math.mulDiv(totalRewards, 1, totalVotes, Math.Rounding.Ceil);
+ rewardPerVoter = Math.mulDiv(totalRewards, 1, totalVotesFor, Math.Rounding.Ceil);
}
_sendEth(s_votersFor[i], rewardPerVoter);
}
}
}
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.