Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: low
Valid

The bug in this code lies in the _distributeRewards() function.

Summary

The contract allows a creator to invite a select group of people to vote on something and provides an ether reward to the for voters if the proposal passes. Otherwise, it refunds the reward to the creator.

The bug in this code lies in the _distributeRewards() function.

Vulnerability Details

The vulnerability is related to the use of integer division. In Solidity, the integer division rounds towards zero, which means that the negative result of division is rounded to a bigger value. This could lead to a situation where the total rewards distributed to the voters is less than the total rewards available, leaving some amount of ether stuck in the contract.

Impact

This vulnerability is problematic because it could lead to a loss of funds.Specifically, the line of code that calculates the reward for each voter uses integer division:

uint256 rewardPerVoter = totalRewards / totalVotes;

This line uses integer division, which could lead to rounding errors. In Solidity, when you divide two integers, the result is also an integer and the remainder is discarded. This could lead to a situation where the total rewards distributed to the voters is less than the total rewards available, leaving some amount of ether stuck in the contract.This line could lead to rounding errors, as the result of the division is truncated, and any remainder is discarded. This could result in the total rewards distributed to the voters being less than the total rewards available, leaving some amount of ether stuck in the contract.

Tools Used

slither

Recommendations

To fix this bug, you should calculate the reward per voter with a more precise method that considers the remainder. Here's an improved version of the _distributeRewards() function using the SafeMath library for arithmetic operations:

import "@openzeppelin/contracts/math/SafeMath.sol";
function _distributeRewards() private {
using SafeMath for uint256;
// the rest of the code remains the same
uint256 totalRewards = address(this).balance;
uint256 remainingRewards = totalRewards;
// ...
for (uint256 i = 0; i < totalVotesFor; ++i) {
uint256 rewardPerVoter;
if (i != totalVotesFor - 1) {
rewardPerVoter = totalRewards.div(totalVotes);
remainingRewards = remainingRewards.sub(rewardPerVoter);
} else {
// for the last voter, give them the remaining rewards to avoid leaving dust
rewardPerVoter = remainingRewards;
}
_sendEth(s_votersFor[i], rewardPerVoter);
}
// ...
}

This version of the _distributeRewards() function uses the SafeMath library to perform arithmetic operations in a safe way that prevents overflow and underflow errors. It also ensures that the last voter gets the remaining rewards, so no ether is left undistributed in the contract.

Updates

Lead Judging Commences

0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

VotingBooth._distributeRewards(): Dust amount can still remain in contract

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.