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

Critical Bug: Incorrect Rewards Distribution Logic

0xtheblackpanther

Summary

A critical bug has been identified in the rewards distribution logic of the smart contract. The issue arises when the proposal is approved, and rewards are intended to be distributed among the "for" voters. However, the current implementation incorrectly uses the total number of voters (both "for" and "against") as the denominator in the reward calculation, leading to improper distribution and potential fund locking within the contract.

Vulnerability Details

Code Location

The issue is located in the distributeRewards function within the smart contract.

Vulnerability Description

The rewards distribution logic incorrectly calculates the denominator for the reward distribution. Instead of considering only the "for" voters, it uses the total number of voters (both "for" and "against"). This leads to a situation where the rewards are distributed among a larger group than intended, causing incorrect fund allocation.

Code snippet

// Incorrect calculation of totalVotes in distributeRewards function
uint256 totalVotesFor = s_votersFor.length;
uint256 totalVotesAgainst = s_votersAgainst.length;
uint256 totalVotes = totalVotesFor + totalVotesAgainst;
​
// Incorrect usage of totalVotes in rewardPerVoter calculation
uint256 rewardPerVoter = totalRewards / totalVotes;

Impact

The impact of this vulnerability is severe. It results in an incorrect distribution of rewards, causing funds to be allocated to a larger group of voters than intended. This can lead to fund locking within the contract and potential financial losses for users.

POC:

  • Copy the below test and add it in VotingBoothTest.t.sol

  • run this command forge test --match-test testVotePassesAndMoneyIsNotSentToAllVoters -vvvv

  • you will get the below result

Result:

Running 1 test for test/VotingBoothTest.t.sol:VotingBoothTest
[PASS] testVotePassesAndMoneyIsNotSentToAllVoters() (gas: 357838)
Logs:
Balance 1 Before: 0
Balance 2 Before: 0
Balance 3 Before: 0
Balance 4 Before: 0
Balance 5 Before: 0
Balance 1 After: 2000000000000000000
Balance 2 After: 0
Balance 3 After: 0
Balance 4 After: 2000000000000000000
Balance 5 After: 2000000000000000000

Test Code:

function testVotePassesAndMoneyIsNotSentToAllVoters() public {
vm.prank(address(0x1));
booth.vote(true);
console.log("Balance 1 Before: ", address(0x1).balance);
​
vm.prank(address(0x2));
booth.vote(false);
console.log("Balance 2 Before: ", address(0x2).balance);
​
vm.prank(address(0x3));
booth.vote(false);
console.log("Balance 3 Before: ", address(0x3).balance);
​
vm.prank(address(0x4));
booth.vote(true);
console.log("Balance 4 Before: ", address(0x4).balance);
​
vm.prank(address(0x5));
booth.vote(true);
console.log("Balance 5 Before: ", address(0x5).balance);
​
console.log("Balance 1 After: ", address(0x1).balance);
console.log("Balance 2 After: ", address(0x2).balance);
console.log("Balance 3 After: ", address(0x3).balance);
console.log("Balance 4 After: ", address(0x4).balance);
console.log("Balance 5 After: ", address(0x5).balance);
​
assert(!booth.isActive() && address(booth).balance > 0);
}

Tools Used

  • Manual review + Fuzzing via foundry.

Recommendations

Update distributeRewards Function: Modify the distributeRewards function to correctly calculate the total number of "for" voters and use this value as the denominator in the reward distribution calculation.

// Correct calculation of totalVotesFor in distributeRewards function
uint256 totalVotesFor = s_votersFor.length;
​
// Correct usage of totalVotesFor in rewardPerVoter calculation
uint256 rewardPerVoter = totalRewards / totalVotesFor;

Also remove the round up check, if this is NOT removed the last voter might receive less reward than others.

-if (i == totalVotesFor - 1) {
- rewardPerVoter = Math.mulDiv(totalRewards, 1, totalVotes, 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.