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

Rewards distributed to FOR voters will be smaller beacuse of mistake in reward calculating.

adamn000

Vulnerability Details

Rewards are distributed to FOR voters only. In VotingBooth::_distributeRewards function totalRewards variable is divided by totalVotes which store number of both FOR and AGAINST voter. Consequently, FOR voters will get less ether than they should. What is more, the rest of funds will be locked in the contract and lost.

PoC

function test_VotersForGetsWrongAmountOfRewards() public {
// ETH for rewards = 10 ETH
vm.prank(address(0x1));
booth.vote(false);
vm.prank(address(0x2));
booth.vote(true);
vm.prank(address(0x3));
booth.vote(true);
​
console.log("Balance voter1 against: ", address(0x1).balance);
console.log("Balance voter2 for: ", address(0x2).balance);
console.log("Balance voter3 for: ", address(0x3).balance);
console.log("Contract balance: ", address(booth).balance);
}

Test result

Balance voter1 against: 0
Balance voter2 for: 3333333333333333333
Balance voter3 for: 3333333333333333334
Contract balance: 3333333333333333333

Impact

User will get smaller rewards than they should. Some funds will be lost.

Tools Used

Foundry

Recommendations

Firstly, totalVotes variable should be changed to totalVotesFor variable. Then, 'if' statement should be removed to calcualte rewards properly.

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);
- }
_sendEth(s_votersFor[i], rewardPerVoter);
}
}
}

Result of test attached in "Vulnerability Details" after changes.

Balance voter1 against: 0
Balance voter2 for: 5000000000000000000
Balance voter3 for: 5000000000000000000
Contract 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.