Cyfrin CodeHawks | #49 - Funds will be trapped in the contract in a situation where all voters do not have same vote

Relevant GitHub Links

https://github.com/Cyfrin/2023-12-Voting-Booth/blob/main/src/VotingBooth.sol#L168-L212

Vulnerability Details

The VotingBooth::_distributeRewards() does not keep track of the contract's balance and does not provide a way for that balance to be withdrawn, resulting in the balance being trapped in the contract forever.

POC

        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;

            for (uint256 i; i < totalVotesFor; ++i) {
                if (i == totalVotesFor - 1) {
                    rewardPerVoter = Math.mulDiv(totalRewards, 1, totalVotes, Math.Rounding.Ceil);
                }
                _sendEth(s_votersFor[i], rewardPerVoter);
            }
        }
    }

The above is the function as provided in the case study. It can be seen that when totalVotesAgainst < totalVotesFor which is the else above, no provision was made to keep track of this balance and also refund the balance back to the owner. The test below when ran also further explains this;

 function testGetBalanceInContract() public {
        vm.prank(address(0x1));
        booth.vote(true);

        vm.prank(address(0x2));
        booth.vote(true);

        vm.prank(address(0x3));
        booth.vote(false);

        assert(booth.getVotersAgainst() < booth.getVotefFor());
        assert(!booth.isActive());
        console.log(address(booth).balance);
    }

when this is ran, it shows the balance in the VotingBooth contract which signifies the amount that would be stuck in the contract. A getVotefFor() and totalVotesAgainst function was added to the VotingBooth and returns the length of voterfor and votedagainst.

Impact

High. Loss of funds.

Tools Used

Manual Review

Recommendations

Keep track of the amount(this amount is the total of those who voted against the proposal), and send this amount to the s_creator.

First Flight #6: Voting Booth

Funds will be trapped in the contract in a situation where all voters do not have same vote

Relevant GitHub Links

Vulnerability Details

POC

Impact

Tools Used

Recommendations

Lead Judging Commences

Rewards breakdown

Live

Judging

Appeals

Appeals Review

Rewards Distribution

FAQs