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

No check to prevent duplicate voter address on the constructor which may lead to funds being locked in the contract permanently

joshuajee

Summary

On the constructor, there is no check to prevent duplicate voter addresses, the total voter
length is dependent on the voter array length this might lead to a situation, where the total number of voters is less than the allowed voters, a situation could occur when the number of allowed voters is less than 51% of total voters, which means the proposal will never pass the minimum voting threshold and funds being permanently trapped.

Vulnerability Details

// cache total voters to prevent multiple storage writes
uint256 totalVoters;
​
// store addresses allowed to vote on this proposal
for (; totalVoters < allowListLength; ++totalVoters) {
// sanity check to prevent address(0) as a valid voter
address voter = allowList[totalVoters];
require(voter != address(0), "DP: address(0) not a valid voter");
​
s_voters[voter] = ALLOWED;
}
​
// update storage of total voters only once
s_totalAllowedVoters = totalVoters;
​

Impact

The proposal maybe doomed to fail from the start and funds might be permanently locked in the contract

POC

Alice deployed the contract with five addresses as voters, but all the addresses were the same,
the smart contract records that the total number of allowed voters is 5, so it is expecting 3 people to vote in order to share the reward.

But the way the contract was built it's one address per vote, so this address can only vote once, so this proposal was doomed from the start.

forge test --mt testInitilizationWithDuplicateAddress
address[] _voters;
function testInitilizationWithDuplicateAddress() public {
deal(address(this), ETH_REWARD);
// allowed voter
_voters.push(address(0x1));
_voters.push(address(0x1));
_voters.push(address(0x1));
_voters.push(address(0x1));
_voters.push(address(0x1));
​
// contracts required for test
VotingBooth _booth = new VotingBooth{value: ETH_REWARD}(_voters);
​
assertEq(_booth.getTotalAllowedVoters(), 5);
​
vm.startPrank(address(0x1));
_booth.vote(true);
​
vm.expectRevert("DP: voter not allowed or already voted");
_booth.vote(true);
​
}

Tool

Foundry and Manual Analysis

Recommendation

Add a check to prevent duplicate addresses on the constructor

address voter = allowList[totalVoters];
require(voter != address(0), "DP: address(0) not a valid voter");
+ require(s_voters[voter] != ALLOWED, "DP: duplicated addresses are not allowed");
Updates

Lead Judging Commences

0xnevi Lead Judge
over 1 year ago
0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Other
joshuajee Submitter
over 1 year ago
0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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