Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

Coding error deviating from documentation results in incorrect voting rewards payout

Summary

Coding error deviating from documentation results in incorrect voting rewards payout

Vulnerability Details

The private function, _distributeRewards(), is supposed to distribute proportional portion of reward amount to voters who voted in favor of the passing proposal. In the two lines specified in Relevant GitHub Links section, the variable totalVotes is used (instead of totalVotesFor) to calculate each "for" voters share of the reward. This results in the payout to the "for" voters being less than what it should be in scenarios where there are "against" voters.

Impact

High - a core function of the protocol works incorrectly

Tools Used

Visual Studio Code, Foundry

Recommendations

Change the _distributeRewards() function as shown below...

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.
uint256 totalRewards = address(this).balance;
// if the proposal was defeated refund reward back to the creator
if (totalVotesAgainst >= totalVotesFor) {
_sendEth(s_creator, totalRewards);
}
// otherwise the proposal passed so distribute rewards to the `For` voters
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);
}
}
}

PoC

Add the following test to the Foundry test file provided by the protocol. It will fail for the current state of codebase and will pass when the recommended changes are made.

function testVotePassesAndMoneyIsSentInCorrectAmounts() public {
vm.prank(address(0x1));
booth.vote(true);
vm.prank(address(0x2));
booth.vote(false);
vm.prank(address(0x3));
booth.vote(true);
// voting closed & money sent
assert(!booth.isActive() && address(booth).balance == 0);
// validate amounts received
// 1 and 3 should get 0.5 eth each
assertEq(
address(0x1).balance,
ETH_REWARD / 2
);
assertEq(
address(0x3).balance,
ETH_REWARD / 2
);
// 2 should get nothing
assertEq(address(0x2).balance, 0);
}
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.