First Flight #6: Voting Booth
First Flight #6
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: low
Valid

Despite Comments Suggesting 1 Wei buffer can be given to the last user, Up to 3 Wei Could Remain in the Contract

0xrektified

Summary

This issue relates to the previously identified problem "Invalid variable used for reward distribution totalVotes instead of totalVotesFor." The initial comment indicated that an extra 1 wei would be left for the last user, but in reality, up to 3 wei could be left in the contract or mistakenly distributed to the last user.

Vulnerability Details

If we apply the current fix:

-> uint256 rewardPerVoter = totalRewards / totalVotesFor;
....
if (i == totalVotesFor - 1) {
-> rewardPerVoter = Math.mulDiv(totalRewards, 1, totalVotesFor, Math.Rounding.Ceil);
}
_sendEth(s_votersFor[i], rewardPerVoter);
​

requires modifying the test as follows to ensure its passage:

function testFuzz_BalanceFocus(uint256 numberOfUser, uint160 valueInETHToFundTheContract) public {
numberOfUser = bound(numberOfUser, 3, 9);
vm.assume(numberOfUser % 2 != 0);
vm.assume(valueInETHToFundTheContract >= 1e18);
deal(address(this), valueInETHToFundTheContract);
setUpFuzz(numberOfUser, valueInETHToFundTheContract);
userVote(numberOfUser, 0);
​
uint256 totalVotesFor = votersFor.length;
console2.log("totalVotesFor");
console2.log(totalVotesFor);
​
if (!booth.isActive()) {
uint256 userFor = booth.getVotersForLength();
uint256 userAgainst = booth.getVotersAgainstLength();
if (userAgainst >= userFor) {
// The contract creator should get all the amount used to fund the contract
require(
address(this).balance == valueInETHToFundTheContract, "post-run: creator should get all funds back"
);
} else {
// Check if all users got their funds back properly
for (uint256 i; i < totalVotesFor; ++i) {
uint256 rewardPerVoter = valueInETHToFundTheContract / totalVotesFor;
if (i == totalVotesFor - 1) {
require(
address(votersFor[i]).balance == rewardPerVoter
|| address(votersFor[i]).balance == rewardPerVoter + 1,
string.concat(
"post-run: last user should have max +1 wei ",
Strings.toString(rewardPerVoter),
" user Balance: ",
Strings.toString(votersFor[i].balance)
)
);
} else {
require(
address(votersFor[i]).balance == rewardPerVoter,
string.concat(
"post-run: each user should have equal share expected: ",
Strings.toString(rewardPerVoter),
" user Balance: ",
Strings.toString(votersFor[i].balance)
)
);
}
}
}
} else {
// Value should still be in the contract as the pool is still active
require(address(booth).balance == valueInETHToFundTheContract, "2");
}
// Contract balance should be zero at the end of the fuzz test
require(
address(booth).balance == 0 || address(booth).balance == 1 || address(booth).balance == 2
|| address(booth).balance == 3,
string.concat("post-run: contract invalid final balance: ", Strings.toString(address(booth).balance))
);
}

Without this condition, the fuzz test consistently fails, indicating that more than 1 wei is left in the contract.

require(
address(booth).balance == 0 || address(booth).balance == 1 || address(booth).balance == 2
|| address(booth).balance == 3,
string.concat("post-run: contract invalid final balance: ", Strings.toString(address(booth).balance))
);

Impact

This finding is considered medium severity. While the comment suggests that only 1 extra wei is given to the last user, in reality, up to 3 wei can be left in the contract. According to Cyfrin FAQ, this classifies as a medium finding

Medium: Funds are indirectly at risk or there's a disruption of protocol functionality or availability.

Tools Used

Manual review

Recommendations

It is important to clarify that a small amount of funds can be stuck in the contract, or alternatively, these funds should be distributed to the last user.

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.