Summary
The rewardPerVoter variable computation within the VotingBooth::_distributeRewards is wrong because uses the totalVotes instead of the totalVotesFor. Due to this miscalculation, the distribution of rewards becomes inaccurate, causing an incorrect allocation of rewards amount to individual users.
Vulnerability Details
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);
}
}
}
Impact
The rewardPerVoter variable computation within the VotingBooth::_distributeRewards is wrong because uses the totalVotes instead of the totalVotesFor. After a vote passes, the rewards are distributed to the for and the contract balance should be 0 but it is not. We can prove them using an invariant test and a simple test.
Proof Of Code
function invariant_balanceSouldBe0WhenInactive() public view {
console.log("Before the contract balance is", address(booth).balance);
if (!booth.isActive()) {
assert(address(booth).balance == 0);
}
console.log("After the contract balance is", address(booth).balance);
}
Failing tests:
Encountered 1 failing test in test/Invariants.t.sol:InvariantsTest
[FAIL. Reason: panic: assertion failed (0x01)]
[Sequence]
sender=0x0000000000000000000000000000000000000007 addr=[src/VotingBooth.sol:VotingBooth]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=vote(bool) args=[false]
sender=0x0000000000000000000000000000000000000003 addr=[src/VotingBooth.sol:VotingBooth]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=vote(bool) args=[true]
sender=0x0000000000000000000000000000000000000007 addr=[src/VotingBooth.sol:VotingBooth]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=vote(bool) args=[false]
sender=0x0000000000000000000000000000000000000001 addr=[src/VotingBooth.sol:VotingBooth]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=vote(bool) args=[false]
sender=0x0000000000000000000000000000000000000004 addr=[src/VotingBooth.sol:VotingBooth]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=vote(bool) args=[true]
sender=0x0000000000000000000000000000000000000007 addr=[src/VotingBooth.sol:VotingBooth]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=vote(bool) args=[true]
sender=0x0000000000000000000000000000000000000008 addr=[src/VotingBooth.sol:VotingBooth]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=vote(bool) args=[true]
The test shows that the vote passes (4 for and 3 against). So the rewards should be distribuited to the for and the balance of the contract must be 0 but the test fails. The balance isn't 0 as expected.
We can prove this also using an easy test for demonstrating that the final balance of the contract isn't 0:
function testVotePassesAndWrongMoneyIsSent() public {
console.log("Initial contract balance", address(booth).balance);
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);
console.log("Final contract balance", address(booth).balance);
}
[PASS] testVotePassesAndWrongMoneyIsSent() (gas: 259958)
Logs:
Initial contract balance 10000000000000000000
Final contract balance 3333333333333333333
Tools Used
Manual review
Recommendations
Use the totalVotesFor instead of the totalVotes in the rewardPerVoter computation.
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);
}
else {
- uint256 rewardPerVoter = totalRewards / totalVotes;
+ uint256 rewardPerVoter = totalRewards / 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);
}
}
}
