President Elector
First Flight #24
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Invalid

Voters array in constructor is unbounded, which could lead to Denial-of-Service (DoS) attacks in contract functions that require looping over `VOTERS` array like `rankCandidates()`, `rankCandidatesBySig()` and `selectPresident()`.

blaze

Description

The larger the RankedChoice::VOTERS array, the more computationally expensive it is to loop through it.

Impact

If the RankedChoice::VOTERS array becomes extremely large causing unfeasible gas costs for calling them, it could render functions like RankedChoice::rankCandidates(), RankedChoice::rankCandidatesBySig() and RankedChoice::selectPresident() uninvokable. This basically blocks all the external functions of contract making it unusable for anyone.

Proof of concept

Add below snippet to RankedChoiceTest.t.sol and run the test case.

function testDoSForLargeCandidatesList() public {
uint256 CANDIDATES_COUNT = 10;
for (uint256 i = 0; i < CANDIDATES_COUNT; i++) {
orderedCandidates.push(address(uint160(i)));
}
​
for (uint256 i = 0; i < 1; i++) {
vm.prank(voters[i]);
rankedChoice.rankCandidates(orderedCandidates);
}
​
vm.warp(block.timestamp + rankedChoice.getDuration()+1);
rankedChoice.selectPresident();
​
uint256 initialGas1 = gasleft();
​
uint256 LESS_VOTERS_COUNT = voters.length - 50;
for (uint256 i = 0; i < LESS_VOTERS_COUNT; i++) {
vm.prank(voters[i]);
rankedChoice.rankCandidates(orderedCandidates);
}
​
vm.warp(block.timestamp + rankedChoice.getDuration()+1);
rankedChoice.selectPresident();
​
uint256 finalGas1 = gasleft();
uint256 gasUsed1 = initialGas1 - finalGas1;
​
console.log("Gas used for less voters: ", gasUsed1);
​
uint256 MORE_VOTERS_COUNT = voters.length;
for (uint256 i = 0; i < MORE_VOTERS_COUNT; i++) {
vm.prank(voters[i]);
rankedChoice.rankCandidates(orderedCandidates);
}
​
vm.warp(block.timestamp + rankedChoice.getDuration()+1);
rankedChoice.selectPresident();
​
uint256 finalGas2 = gasleft();
uint256 gasUsed2 = finalGas1 - finalGas2;
​
console.log("Gas used for more voters: ", gasUsed2);
​
assert(gasUsed2 > gasUsed1);
}
forge test --mt testDoSForLargeCandidatesList -vvv
# Gas used for less voters: 16721386
# Gas used for more voters: 32283120

Recommended Mitigation

An upper limit can be set when passing voters array in the constructor.

uint256 private constant MAX_CANDIDATES = 10;
+ uint8 private constant MAX_VOTERS = 100;
constructor(address[] memory voters) EIP712("RankedChoice", "1") {
+ if (voters.length > MAX_VOTERS) {
+ revert RankedChoice__InvalidInput();
+ }

Tools used

  • Foundry

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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