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

[L-1] No checks for address(0) in `rankCandidates` could potentially lead to no one winning the election

JJS

Description

Voters can call the rankCandidates external function, or the rankCandidatesBySig function, that calls an internal function to rank the candidates (_rankCandidates). The voter passes through an ordered candidate list, with their ranking.

However, there is no checks for an address(0) in rankCandidates or _rankCandidates. In the case of rankCandidatesBySig, the ECDSA.recover function calls an internal function in the ECDSA contract that does check for an address(0), so this does not apply.

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

Impact

LOW. This is because the likelihood of voters actually voting for an `address(0)` is very low. However, This could mean that, potentially, an empty address could win the election, and therefore no one is the actual president!

Proof of Concept

In this little test I prove that a voter can pass through an `address(0)` to the `s_rankings` mapping: (I also added the attacker address to the setUp to be added to VOTERS).

function testToAddAddress0ToCandidateList() public {
address[] memory myOrder = new address[]();
myOrder[0] = address(0);
vm.startPrank(attacker);
rankedChoice.rankCandidates(myOrder);
}

Recommended Mitigation

Have a for loop that checks for address(0) in the `_rankCandidates`:

```diff
​
+ error RankedChoice__CandidateCannotBeAddress0;
​
function _rankCandidates(address[] memory orderedCandidates, address voter) internal {
// Checks
if (orderedCandidates.length > MAX_CANDIDATES) {
revert RankedChoice__InvalidInput();
}
if (!_isInArray(VOTERS, voter)) {
revert RankedChoice__InvalidVoter();
}
​
+ for (uint256 i=0; i<orderedCandidates.length; i++){
+ if(orderedCandidates[i] == address(0)){
+ revert RankedChoice__CandidateCannotBeAddress0();
+ }
+ }
​
// Internal Effects
s_rankings[voter][s_voteNumber] = orderedCandidates;
}
​
```
Updates

Lead Judging Commences

inallhonesty Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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