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

Incorrect calculation of rewardPerVoter when the proposal passes will result in users receiving less rewards and the differnce will be stuck in the contract.

Summary

_distributeRewards() serves the purpose of transferring Ether within the contract to the owner if the proposal fails, or to the For voters if the proposal succeeds. However, there's an issue in the calculation of rewardPerVoter, resulting in an incorrect distribution. This miscalculation leads to a situation where users who vote For receive fewer rewards.

Vulnerability Details

distributeRewards() calculates the rewardPerVoter using a formula that involves totalVotes when a proposal passes.

uint256 totalVotesFor = s_votersFor.length;
uint256 totalVotesAgainst = s_votersAgainst.length;
uint256 totalVotes = totalVotesFor + totalVotesAgainst;
uint256 rewardPerVoter = totalRewards / totalVotes;

While this approach works seamlessly when all votes are in favor, it introduces a problem when there's even a single vote against. In such cases, for each against vote, the users who voted For will receive less rewards.

For better comprehension, let's illustrate with an example:

---------------------------Example---------------------------

  • totalVotesFor = 3

  • totalVotesAgainst = 1

  • totalVotes = 3 + 1 = 4

  • totalRewards = 4 ETH

  • rewardPerVoter = 4 / 4 = 1 ETH

Each user will receive 1 ETH provocating a 1 ETH becomes trapped within the contract. The accurate totalRewards should be:

  • rewardPerVoter = 4 / 3 = 1.3 ETH

POC

function testVotePassesButAlltheMoneyIsNotSent() 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);
assertEq(address(booth).balance, ETH_REWARD / 3); //Eth stuck in the contract
assertEq(address(0x1).balance, ETH_REWARD / 3);
assertEq(address(0x2).balance, ETH_REWARD / 3 + 1);
}

Impact

Users who vote For will receive less rewards, and the remaining difference will become trapped within the contract.

Tools Used

Manual review.

Recommendations

Change rewardPerVoter for the following formula rewardPerVoter = totalRewards / totalVotesFor.

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.