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

Dust calculation is wrong

Summary

Dust calculation is wrong when dust is greater than 1 WEI.

Vulnerability Details

When totalRewards - rewardPerVoter * totalVotesFor is greater than 1 WEI some WEI are left on the contract. For example when allowing 9 voters, after 5 afirmative votes and with a reward not divisible by 5.

Impact

Some WEI are left on the contract, which are lost, since there is no way to retrieve them.

Tools Used

  • forge test

Failing test:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.23;
import {VotingBooth} from "../src/VotingBooth.sol";
import {Test} from "forge-std/Test.sol";
contract VotingBoothDustTest is Test {
// eth reward
uint256 constant ETH_REWARD = 10e18 + 3;
// allowed voters
address[] voters;
// contracts required for test
VotingBooth booth;
function setUp() public virtual {
// deal this contract the proposal reward
deal(address(this), ETH_REWARD);
// setup the allowed list of voters
voters.push(address(0x1));
voters.push(address(0x2));
voters.push(address(0x3));
voters.push(address(0x4));
voters.push(address(0x5));
voters.push(address(0x6));
voters.push(address(0x7));
voters.push(address(0x8));
voters.push(address(0x9));
// setup contract to be tested
booth = new VotingBooth{value: ETH_REWARD}(voters);
// verify setup
//
// proposal has rewards
assert(address(booth).balance == ETH_REWARD);
// proposal is active
assert(booth.isActive());
// proposal has correct number of allowed voters
assert(booth.getTotalAllowedVoters() == voters.length);
// this contract is the creator
assert(booth.getCreator() == address(this));
}
// required to receive refund if proposal fails
receive() external payable {}
function testNoDustLeft() public {
vm.prank(address(0x1));
booth.vote(true);
vm.prank(address(0x2));
booth.vote(true);
vm.prank(address(0x3));
booth.vote(true);
vm.prank(address(0x4));
booth.vote(true);
vm.prank(address(0x5));
booth.vote(true);
assert(address(booth).balance == 0);
}
}

Recommendations

Since we are sending the funds in the loop, by the time we reach the last for voter only their reward plus the dust are left on the contract, then we can send them all the account balance replacing line 207 by:

rewardPerVoter = address(this).balance;

Notice

This issue was given Low severity since at most MAX_VOTERS - 2 WEI are lost.
This issue was reported as a separate issue, since fixing the reward calculation would not fix this other issue.

Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year 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.