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

`rewardPerVoter` is not calculated properly causing funds to be inaccurately distributed and permanently locked in the contract

joshuajee

Summary

The rewardPerVoter was miscalculated twice, instead of dividing by totalVotesFor, it was divided by totalVotes, this caused the reward shared amongst voters to be lesser than required and it also permanently locked the remaining funds in the contract.

Vulnerability Details

The rewardPerVoter was miscalculated, instead of dividing by totalVotesFor, it was divided by totalVotes, this causes the reward shared amongst voters to be lesser than required thereby permanently locking the remaining funds in the contract, the same thing happens in the last instance, it shortchanged the last voter and locked the remaining funds, permanently in the contract.

function _distributeRewards() private {
// get number of voters for & against
uint256 totalVotesFor = s_votersFor.length;
uint256 totalVotesAgainst = s_votersAgainst.length;
uint256 totalVotes = totalVotesFor + totalVotesAgainst;
​
// rewards to distribute or refund. This is guaranteed to be
// greater or equal to the minimum funding amount by a check
// in the constructor, and there is intentionally by design
// no way to decrease or increase this amount. Any findings
// related to not being able to increase/decrease the total
// reward amount are invalid
uint256 totalRewards = address(this).balance;
​
// if the proposal was defeated refund reward back to the creator
// for the proposal to be successful it must have had more `For` votes
// than `Against` votes
if (totalVotesAgainst >= totalVotesFor) {
// proposal creator is trusted to create a proposal from an address
// that can receive ETH. See comment before declaration of `s_creator`
_sendEth(s_creator, totalRewards);
}
// otherwise the proposal passed so distribute rewards to the `For` voters
else {
@> 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);
}
}
}

Impact

Permanently Lock funds in the protocol, and also sends less rewards to voters

POC

Below is the proof of concept the last assert will fail because funds is still locked in the protocol, even after rewards have been shared

forge test --match-test testVotePassesWithOneVoteAgaint

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

Tools Used

Foundry and Manual Analysis

Recommendations

Replace totalVotes with totalVotesFor in both occurrence

- uint256 rewardPerVoter = totalRewards / totalVotes;
+ uint256 rewardPerVoter = totalRewards / totalVotesFor;
- uint256 rewardPerVoter = Math.mulDiv(totalRewards, 1, totalVotes, Math.Rounding.Ceil);
+ uint256 rewardPerVoter = Math.mulDiv(totalRewards, 1, totalVotesFor, Math.Rounding.Ceil);
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.