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

Unbound loops over voters and candidates arrays can lead to Gas Limit DoS

aeff

Relevant GitHub Links

https://github.com/Cyfrin/2024-09-president-elector/blob/main/src/RankedChoice.sol#L60
https://github.com/Cyfrin/2024-09-president-elector/blob/main/src/RankedChoice.sol#L98

Summary

The selectPresident** function makes iteration over the voters array and candidates array which can lead to "Out of Gas" revert and eventually denial of service.

Vulnerability Details

The rankChoice::selectPresident and rankChoice::_selectPresident functions make iteration over the voters array and candidates array. Due to the nature of such arrays which can be very large for both voters and candidates, iterating over such arrays can exceed current block's gas limit and leads to "Out of Gas" error. Eventually, this will break the "selectPresident" functionality as it will always revert and make the contract useless.

In addition, the "_rankcandidates" function uses the "_isInArray" function to check if the voter is within the voters list or not. If the Voters array is big, then this can lead to Gas Limit DoS preventing users from voting.

The following code snippets can lead to Dos:

1- The _isInArray(VOTERS, voter) function call inside "_rankCandidates" if the VOTERS array is very big:

function _rankCandidates(
address[] memory orderedCandidates,
address voter
) internal {
// if (s_votings[voter][s_voteNumber]) {
// revert RankedChoice__VoterAlreadyVoted();
// }
// Checks
if (orderedCandidates.length > MAX_CANDIDATES) {
revert RankedChoice__InvalidInput();
}
if (!_isInArray(VOTERS, voter)) {
revert RankedChoice__InvalidVoter();
}
​
// Internal Effects
s_rankings[voter][s_voteNumber] = orderedCandidates;
s_votings[voter][s_voteNumber] = true;
}

2- Nested loop inside selectPresident function:

for (uint256 i = 0; i < VOTERS.length; i++) {
address[] memory orderedCandidates = s_rankings[VOTERS[i]][
s_voteNumber
];
for (uint256 j = 0; j < orderedCandidates.length; j++) {
if (!_isInArray(s_candidateList, orderedCandidates[j])) {
s_candidateList.push(orderedCandidates[j]);
}
}
}

3- Nested loop inside _selectPresident::

for (uint256 i = 0; i < VOTERS.length; i++) {
for (
uint256 j = 0;
j < s_rankings[VOTERS[i]][s_voteNumber].length;
j++
) {
address candidate = s_rankings[VOTERS[i]][s_voteNumber][j];
if (_isInArray(candidateList, candidate)) {
s_candidateVotesByRound[candidate][s_voteNumber][
roundNumber
] += 1;
total_votes += 1;
break;
} else {
continue;
}
}
}

4- Other loops inside _selectPresident:

for (uint256 i = 1; i < candidateList.length; i++) {
uint256 votes = s_candidateVotesByRound[candidateList[i]][
s_voteNumber
][roundNumber];
if (votes > highestVotes) {
highestVotes = votes;
highestVotesCandidate = candidateList[i];
}
}
for (uint256 i; i < candidateList.length; i++) {
if (passedCandidate) {
newCandidateList[i - 1] = candidateList[i];
} else if (candidateList[i] == fewestVotesCandidate) {
passedCandidate = true;
} else {
newCandidateList[i] = candidateList[i];
}
}

Impact

The main impact of this issue is a Denial of Service (DoS) which breaks the selectPresident function. This prevents the contract from selecting the winning candidate as the president after the end of the voting duration.

Tools Used

Manual Code Review and Foundry Unit Test

Recommendations

To mitigate the risk of a denial-of-service attack due to an unbounded loop, consider the following approaches:

  1. Batch Processing: Implement batch processing to distribute rewards in smaller batches over multiple transactions. This would prevent the function from exceeding the block gas limit.

  2. Mappings: Use mappings instead of arrays whenever possible.

  3. Gas Limit Checks: Introduce a check for the gas limit before the loop execution and halt the operation if the number of claimants is too large to process in a single transaction.

Updates

Lead Judging Commences

inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

A high number of candidates could cause an OOG

Support

FAQs

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