Summary
There is a flawed implementation in rewardPerVoter that causes conflicts in many combinations, resulting in a poor distribution of rewards. This is exacerbated a little bit by the use of Math.mulDiv(totalRewards, 1, totalVotes, Math.Rounding.Ceil).
Vulnerability Details
The line 192 is much more severe as it divides the rewards by totalVotes, causing a significant dilution of rewards in most situations where distribution to users is required.
The line 207 is less critical, leaving at least one wei remaining in some combinations. Rounding to a single wei for the last user is sometimes insufficient, breaking the invariant test.
Impact
Funds locked in the contract may become unrecoverable, leading to significant losses.
POC
It may be necessary to make some VotingBooth.sol storage public again.
Handler:
pragma solidity 0.8.23;
import {Test, console} from "forge-std/Test.sol";
import {VotingBooth} from "../../src/VotingBooth.sol";
contract Handler is Test {
struct HandlerProps {
VotingBooth votingBooth;
address[] allowList;
}
HandlerProps state;
address[] forVoters;
constructor(HandlerProps memory props){
state.votingBooth = props.votingBooth;
for(uint i = 0; i < props.allowList.length; i++){
state.allowList.push(props.allowList[i]);
}
}
function vote(bool _vote, uint256 _voterSeed) external {
uint index = bound(_voterSeed, 0, state.allowList.length - 1);
address voter = state.allowList[index];
if(state.votingBooth.s_voters(voter) != 1) return;
if(!state.votingBooth.isActive()) return;
if(_vote){
forVoters.push(voter);
}
vm.startPrank(voter);
state.votingBooth.vote(_vote);
vm.stopPrank();
}
function getForVoters(uint256 index) external view returns (address){
return forVoters[index];
}
function getForVotersAmount() external view returns (uint256){
return forVoters.length;
}
}
Invariant:
pragma solidity 0.8.23;
import {Test, StdInvariant, console} from "forge-std/Test.sol";
import {Handler} from "./Handler.t.sol";
import {VotingBooth} from "../../src/VotingBooth.sol";
contract VotingBoothInvariant is StdInvariant, Test {
struct SetUpConfig {
address OWNER;
uint256 totalAllowedVoters;
uint256 REWARD;
}
struct VotingBoothProps {
address[] allowList;
}
SetUpConfig config;
VotingBoothProps votingBoothProps;
Handler handler;
VotingBooth votingBooth;
function setUp() external {
config.OWNER = makeAddr("OWNER");
config.REWARD = 5 ether;
vm.deal(config.OWNER, config.REWARD);
config.totalAllowedVoters = 7;
for(uint256 i = 0; i < config.totalAllowedVoters; i++){
votingBoothProps.allowList.push(address(uint160(i+1)));
}
vm.startPrank(config.OWNER);
votingBooth = new VotingBooth{value: config.REWARD}(votingBoothProps.allowList);
vm.stopPrank();
Handler.HandlerProps memory handlerProps = Handler.HandlerProps({votingBooth: votingBooth, allowList: votingBoothProps.allowList});
handler = new Handler(handlerProps);
bytes4[] memory selectors = new bytes4[](1);
selectors[0] = handler.vote.selector;
targetSelector(FuzzSelector({ addr: address(handler), selectors: selectors }));
targetContract(address(handler));
}
function invariant_distributedVotingBoothBalance() external {
if(votingBooth.isActive()){
return;
}
assert(address(votingBooth).balance == 0);
uint256 totalForVoters = handler.getForVotersAmount();
if(((config.totalAllowedVoters / 2) + 1) / 2 < totalForVoters){
uint256 distributedReward = config.REWARD / totalForVoters;
for(uint256 i = 0; i < totalForVoters; i++){
address voter = handler.getForVoters(i);
assert(voter.balance >= distributedReward);
}
}
else{
assert(config.OWNER.balance == config.REWARD);
}
}
}
Tools Used
Foundry
Recommendations
- uint256 rewardPerVoter = totalRewards / totalVotes;
+ uint256 rewardPerVoter = totalRewards / totalVotesFor;
- rewardPerVoter = Math.mulDiv(totalRewards, 1, totalVotes, Math.Rounding.Ceil);
+ rewardPerVoter = address(this).balance;
