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

‘For’ Voters Might Not Receive the Correct Amount of Reward

Summary

The smart contract's _distributeRewards() function exhibits a critical flaw in its reward distribution logic. Specifically, it miscalculates the distribution of funds by dividing the total available rewards not by the count of approving ('for') votes but by the combined tally of all votes, diluting the reward per approving participant. This not only results in a less-than-expected payout for voters who supported the proposal but also leads to a surplus of undistributed funds remaining within the contract.

Vulnerability Details

If everyone who votes are 'for' voters, the rewards calculated would be accurate as the total number of voters are all 'for' voters. However, the problem arises when the 'against' votes are present but there are enough 'for' votes for the proposal to pass.

The reward will be calculated with the assumption that it will be distributed to all the voters, but only distributing to the 'for' voters, leading to a mismatch between the calculated and actual reward distribution.

Imagine a scenario similar to the test provided.

There are in total 5 voters in the allowedList. Now, in order for it to pass and distribute the rewards, 3 out of 5 needs to vote. The number of 'for' voters need to be more than 'against' voters too. The first voter votes 'against' it, while the second and third voter vote 'for'. It passes the MIN_QUORUM which will then call _distributeRewards(), and it will run the else statement which is to distribute the rewards to all the 'for' voters.

However, the flaw lies in line 192, where it calculates how much of the rewards is to be distributed. It calculates the rewardPerVoter by dividing the totalRewards by totalVotes which is inclusive of the number of voters that are against it. This means that it will be divided by 3. Then, it loops through the number of totalVotesFor which is 2, and distribute accordingly. However, the total distributed will be less than the actual amount since it was calculated for 3 people but only distributed to 2 of the 'for' voters, leading to undistributed funds in the contract.

Example

Say there is 120 ETH in the VotingBooth contract, where one voter opposes and two support the proposal, the contract will proceed and distribute the rewards to the 'for' voters.

The rewardPerVoter will be incorrectly calculated as 120 ETH / 3 votes = 40 ETH per voter, leading to only 80 ETH being distributed. The proper calculation should reflect 120 ETH / 2 votes = 60 ETH per voter.

Impact

The impact of this vulnerability as it results in 'for' voters receiving less ETH than intended, undermining the trust in the protocol.

Proof Of Concept

Add the test to the file: test/VotingBoothTest.t.sol.

Run the test:

forge test --mt "testFundsAreNotProperlyDistributed"
function testFundsAreNotProperlyDistributed() public {
// Vote 'against'
vm.prank(address(0x1));
booth.vote(false);
vm.prank(address(0x2));
booth.vote(true);
vm.prank(address(0x3));
booth.vote(true);
// Get balance
uint256 address2Balance = address(0x2).balance;
uint256 address3Balance = address(0x3).balance;
uint256 totalAddressesBalance = address2Balance + address3Balance;
// Contract balance is not fully distributed
assertNotEq(address(booth).balance, 0);
// the 'for' voters did not get the full reward
assertNotEq(totalAddressesBalance, ETH_REWARD);
}

Tools Used

Manual Review, Foundry

Recommendations

Fix the calculation flaw by changing the variable that totalRewards is divided by.

- uint256 rewardPerVoter = totalRewards / totalVotes;
+ uint256 rewardPerVoter = totalRewards / totalVotesFor;

Change the calculation for the last 'for' voter too

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