Submission Details
Severity:
wrong rewards distribution
Summary
There is a bug in the VotingBooth::_distributeRewards function, when the proposal passes the value of rewardPerVoter is calculated incorrectly, dividing totalRewards by totalVotes instead of totalVotesFor as suggested by the comment.
Vulnerability Details
// otherwise the proposal passed so distribute rewards to the `For` voters
else {
@> uint256 rewardPerVoter = totalRewards / totalVotes;
for (uint256 i; i < totalVotesFor; ++i) {
// proposal creator is trusted when creating allowed list of voters,
// findings related to gas griefing attacks or sending eth
// to an address reverting thereby stopping the reward payouts are
// invalid. Yes pull is to be preferred to push but this
// has not been implemented in this simplified version to
// reduce complexity & help you focus on finding the
// harder to find bug
// if at the last voter round up to avoid leaving dust; this means that
// the last voter can get 1 wei more than the rest - this is not
// a valid finding, it is simply how we deal with imperfect division
if (i == totalVotesFor - 1) {
rewardPerVoter = Math.mulDiv(totalRewards, 1, totalVotes, Math.Rounding.Ceil);
}
_sendEth(s_votersFor[i], rewardPerVoter);
}
}
By dividing totalRewards incorrectly and then distributing the value to the users who voted in favor, it happen that part of the total reward remains in the balance of the contract, and the users who voted in favor receive a lower value than what it's up to him.
Here a test to prove that:
Code
function testVotePassesMoneyIsSentNotAll() public {
console.log("Total amount of rewards: 10 eth");
console.log(address(booth).balance / (1 ether));
console.log(
"There will be 2 winners, so the rewards will be 5 eth each"
);
console.log((address(booth).balance / 2) / (1 ether));
vm.prank(address(0x1));
booth.vote(true);
vm.prank(address(0x2));
booth.vote(false);
vm.prank(address(0x3));
booth.vote(true);
console.log("Address 0x1 balance");
console.log(address(0x1).balance / (1 ether));
console.log("Contract balance after rewards distribution");
console.log(address(booth).balance / (1 ether));
assert(!booth.isActive());
assert(address(booth).balance == 0);
}
Logs:
Logs:
Total amount of rewards: 10 eth
10
There will be 2 winners, so the rewards will be 5 eth each
5
Address 0x1 balance
3
Contract balance after rewards distribution
3
Impact
Users who voted in favor receive a smaller sum than agreed upon and a sum remains in the balance.
Tools Used
Manual review and Foundry test.
Recommendations
The function code must be modified to use totalVotesFor as a dividend and the sending of the sum to the last user must also be reviewed so that it takes all the remaining balance avoiding leaving any gwei in the balance.
function _distributeRewards() private {
// get number of voters for & against
uint256 totalVotesFor = s_votersFor.length;
uint256 totalVotesAgainst = s_votersAgainst.length;
- uint256 totalVotes = totalVotesFor + totalVotesAgainst;
// rewards to distribute or refund. This is guaranteed to be
// greater or equal to the minimum funding amount by a check
// in the constructor, and there is intentionally by design
// no way to decrease or increase this amount. Any findings
// related to not being able to increase/decrease the total
// reward amount are invalid
uint256 totalRewards = address(this).balance;
// if the proposal was defeated refund reward back to the creator
// for the proposal to be successful it must have had more `For` votes
// than `Against` votes
if (totalVotesAgainst >= totalVotesFor) {
// proposal creator is trusted to create a proposal from an address
// that can receive ETH. See comment before declaration of `s_creator`
_sendEth(s_creator, totalRewards);
}
// otherwise the proposal passed so distribute rewards to the `For` voters
else {
- uint256 rewardPerVoter = totalRewards / totalVotes;
+ uint256 rewardPerVoter = totalRewards / totalVotesFor;
for (uint256 i; i < totalVotesFor; ++i) {
// proposal creator is trusted when creating allowed list of voters,
// findings related to gas griefing attacks or sending eth
// to an address reverting thereby stopping the reward payouts are
// invalid. Yes pull is to be preferred to push but this
// has not been implemented in this simplified version to
// reduce complexity & help you focus on finding the
// harder to find bug
// if at the last voter round up to avoid leaving dust; this means that
// the last voter can get 1 wei more than the rest - this is not
// a valid finding, it is simply how we deal with imperfect division
if (i == totalVotesFor - 1) {
- rewardPerVoter = Math.mulDiv(
- totalRewards,
- 1,
- totalVotes,
- Math.Rounding.Ceil
- );
+ rewardPerVoter = address(this).balance;
}
_sendEth(s_votersFor[i], rewardPerVoter);
}
}
}
Updates
Lead Judging Commences
0xnevi over 1 year ago
Submission Judgement Published Assigned finding tags:
VotingBooth._distributeRewards(): Incorrect computation of rewardPerVoter
Rewards breakdown
This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.