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

ETH Rewards Permanently Locked in the Contract

FrostBurnXI

Summary

When a proposal successfully achieves quorum in the VotingBooth contract, a certain portion of the reward is inappropriately locked within the contract.

Vulnerability Details

This issue occurs when the quorum is met for a successful proposal in the VotingBooth contract and totalVotesAgainst is greater than zero causing an amount of Ether (ETH) proportional to totalVotesAgainst to not be distributed to the participants that voted in favor of the proposal. The root cause is traced to the calculation method of rewardPerVoter within the VotingBooth::_distributeRewards() function, where rewardPerVoter is calculated using totalVotes as the divisor instead of totalVotesFor, causing the rewardPerVoter to be too low.

In contrast, for a failed proposal reaching quorum, the entire rewardAmount is appropriately returned to VotingBooth::s_creator.

Stateful Fuzzing Test

The following stateful fuzzing test demonstrates that there exists a sequence of votes where, upon reaching quorum, the contract still possesses ETH, which it should not.

/**
* @dev This test checks if, in a scenario where a quorum is reached,
* the contract still holds ETH when it should not.
*/
function invariant_testQuorumReachedDoesNotLeaveETHInBooth() public {
// Check if the voting booth is inactive
if (!booth.isActive()) {
// If inactive, ensure the contract's balance is 0
assert(address(booth).balance == 0);
}
}

Output:

​
[FAIL. Reason: panic: assertion failed (0x01)]
[Sequence]
sender=0x0000000000000000000000000000000000000002 addr=[src/VotingBooth.sol:VotingBooth]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=vote(bool) args=[true]
sender=0x0000000000000000000000000000000000000001 addr=[src/VotingBooth.sol:VotingBooth]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=vote(bool) args=[true]
sender=0x0000000000000000000000000000000000000003 addr=[src/VotingBooth.sol:VotingBooth]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=vote(bool) args=[false]
invariant_testQuorumReachedDoesNotLeaveETHInBooth() (runs: 256, calls: 3832, reverts: 3064)

These issues prompted the creation of the following unit tests, which demonstrate that the remaining balance is caused by votes against the proposal.

Unit Tests

Test 1: Votes Against Proposal Result in Remaining ETH

This test examines the behavior of the booth contract when a proposal passes but has at least one vote against it. In such a scenario, if the bug is present, the contract will not correctly send the ETH to the "For" voters, and the contract will still have ETH in it.

function testVotesAgainstProposalCauseEthToRemainInBooth() public {
// Cast a vote using `vm.prank` for address(0x1)
vm.prank(address(0x1));
booth.vote(true);
​
// Cast a vote using `vm.prank` for address(0x2)
vm.prank(address(0x2));
booth.vote(true);
​
// Cast a vote using `vm.prank` for address(0x3)
vm.prank(address(0x3));
booth.vote(false);
​
// Check if the voting booth is no longer active
assert(!booth.isActive());
​
// Check if the contract balance is greater than 0
assert(address(booth).balance > 0);
}

Test 2: All Votes For Proposal Leave No ETH in Booth

This passing test examines the behavior of the booth contract when all votes are cast in favor of the proposal. In this case, the contract should not have any ETH remaining because the number of "For" votes equals the total number of voters.

function testAllVotesForProposalLeavesNoEthInBooth() public {
// Cast a vote using `vm.prank` for address(0x1)
vm.prank(address(0x1));
booth.vote(true);
​
// Cast a vote using `vm.prank` for address(0x2)
vm.prank(address(0x2));
booth.vote(true);
​
// Cast a vote using `vm.prank` for address(0x3)
vm.prank(address(0x3));
booth.vote(true);
​
// Check if the voting booth is no longer active
assert(!booth.isActive());
​
// Check if the contract balance is greater than 0
assert(address(booth).balance > 0);
}

These unit tests help identify issues related to the contract's handling of ETH and voting outcomes. Specifically showing how votes Against cause ETH to remain permanently locked in the contract.

Looking at a snippet from VotingBooth::_distributeRewards():

if (totalVotesAgainst >= totalVotesFor) {
_sendEth(s_creator, totalRewards);
}
else {
uint256 rewardPerVoter = totalRewards / totalVotes; // here we see totalVotes is used instead of totalVotesFor
for (uint256 i; i < totalVotesFor; ++i) {
if (i == totalVotesFor - 1) {
rewardPerVoter = Math.mulDiv(totalRewards, 1, totalVotes, Math.Rounding.Ceil);
}
_sendEth(s_votersFor[i], rewardPerVoter);
}
}

We can see that in first line in the else clause that when rewardPerVoter is calculated totalVotes is used to split the reward instead of totalVotesFor which is the root cause that causes Eth to remain stranded in the contract.

Impact

This flaw affects all participants eligible for rewards. As long as there are votes against a proposal, a portion of the reward ETH, proportional to totalVotesAgainst, will invariably remain locked in the contract. Significantly, the absence of a public or external function to retrieve this ETH means that the locked funds become permanently inaccessible.

Tools Used

  • Invariant Testing with Forge

  • Comprehensive Unit Testing

  • In-depth Manual Code Review

Recommendations

To ensure proper distribution of rewards, it is advised to modify the calculation of rewardPerVoter. This can be achieved by dividing the totalRewards by totalVotesFor, rather than totalVotes. The suggested code alteration is as follows:

// Adjusted reward calculation formula
+ uint256 rewardPerVoter = totalRewards / totalVotesFor;
// Original, flawed reward calculation formula
- uint256 rewardPerVoter = totalRewards / totalVotes;
​
Updates

Lead Judging Commences

0xnevi Lead Judge almost 2 years 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.