Summary
The rewardPerVoter amount of the last voter in the 'for' voters list is incorrect. The rewardPerVoter parameter computation within the VotingBooth::_distributeRewards is wrong because uses the totalVotes instead of the totalVotesFor. Due to this miscalculation, the distribution of rewards of the last voter becomes wrong.
Note: also the rewardPerVoter calculation of all "for" voters is incorrect as indicated in the "Incorrect calculation of the rewardPerVoter within the VotingBooth::_distributeRewards computation leads to inaccurate distribution of rewards" findings. Please refer to this finding for more details.
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 of the last voter is wrong. After a vote passes, an equal reward is distributed to the for (less than 1 Wei) and the contract balance should be 0 but it is not.
Proof Of Code
function testVotePassesAndWrongMoneyIsSentToTheLastVoters() 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);
console.log("User 1 balance", address(0x1).balance);
console.log("User 2 balance", address(0x2).balance);
assert(!booth.isActive() && address(booth).balance != 0);
console.log("Final contract balance", address(booth).balance);
}
Logs:
Initial contract balance 10000000000000000000
User 1 balance 5000000000000000000
User 2 balance 3333333333333333334
Final contract balance 1666666666666666666
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;
for (uint256 i; i < totalVotesFor; ++i) {
if (i == totalVotesFor - 1) {
- rewardPerVoter = Math.mulDiv(totalRewards, 1, totalVotes, Math.Rounding.Ceil);
+ rewardPerVoter = Math.mulDiv(totalRewards, 1, totalVotesFor, Math.Rounding.Ceil);
}
_sendEth(s_votersFor[i], rewardPerVoter);
}
}
}
