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

Wrong formula for calculating the reward distribution amount to voters which votes in favour of proposal

Summary

The contract has a vulnerability in the reward distribution logic when the proposal passes. If there are more in-favor votes than against votes, the contract incorrectly calculates the reward amount for each in-favor voter. This leads to an inaccurate distribution of funds, potentially leaving some amount within the contract.

Vulnerability Details

When the contract calculates the reward amount for distribution to in-favor voters, it uses the total number of votes instead of the total number of in-favor votes. This results in an incorrect calculation, and a portion of the total reward may remain within the contract after distribution.

For example : if there are 10 ETH in total rewards and 3 votes in total (2 in favor, 1 against), the incorrect calculation would distribute rewards as if there were 3 voters instead of 2. This leads to an incomplete distribution of the total reward amount.

                 rewardPerVoter = totalRewards/ totalVotes ;
                 So here :  rewardPerVoter = 10 / 3;

It means the first address get the 3eth and second one gets the 4eth.Means the total reward amount which is distributed is 7eth instead of total reward
amount which is 10eth.

Code Snippet :

       // otherwise the proposal passed so distribute rewards to the `For` voters
       else {
       //@audit : totalRewards must be divide by totalVotesFor instead of totalVotes.
        uint256 rewardPerVoter = totalRewards / totalVotes;
       for (uint256 i; i < totalVotesFor; ++i) {
            if (i == totalVotesFor - 1) {
        // @audit : use totalVotesFor instead of totalVotes here .
                rewardPerVoter = Math.mulDiv(totalRewards, 1, totalVotes, Math.Rounding.Ceil);
            }
            _sendEth(s_votersFor[i], rewardPerVoter);
        }
    }

POC

    function test_audit_VotePassesAndMoneyIsSent() 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() && address(booth).balance == 0);
}

Impact

The impact of this vulnerability is that the reward amount for in favor voters is miscalculated based on the total number of votes, potentially leaving some reward funds within the contract after distribution.

Tools Used

Manual check , Foundry

Recommendations

Write the code like this.

function _distributeRewards() private {
    // get number of voters for & against
    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);
    }
    // otherwise the proposal passed so distribute rewards to the `For` voters
    else {
        // @audit - correction : write like this
        uint256 rewardPerVoter = totalRewards / totalVotesFor;

        for (uint256 i; i < totalVotesFor; ++i) {
            if (i == totalVotesFor - 1) {
                // @audit - correction : write like this
                rewardPerVoter = Math.mulDiv(totalRewards, 1, totalVotesFor, Math.Rounding.Ceil);
            
            }
            _sendEth(s_votersFor[i], rewardPerVoter);
        }
    }
}
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.