Relevant GitHub Links
https://github.com/Cyfrin/2024-09-president-elector/blob/fccb8e2b6a32404b4664fa001faa334f258b4947/src/RankedChoice.sol#L159
Summary
Voters can change their vote many times within the same voting cycle without any restriction which breaks a core principle in the RCV voting system.
Vulnerability Details
The RankedChoice::_rankCandidate function does not have any restriction to prevent voters from changing their votes any time within the voting duration. This breaks the one of the core principles of the RCV voting system, makes the voting volatile, and votes can be changed over time based on trends or orientation to specific candidates which leads to unfair and unexpected results.
Impact
If voters can vote any times without any restriction, this can impact the voting system in many ways as follows:
1- It breaks a core principle in the RCV system where voters must only vote once per voting cycle.
2- It might encourage strategic voting. Voters might switch their votes to favor a candidate more likely to win, or to block another candidate from winning.
3- It makes the voting very volatile and the end results are not reliable.
Proof of Concept
To demonstrate the issue, add the following test in RankedChoiceTest.t.sol, and execute the following
forge test --mt testVoteMultipleTimes -vv
Unit Test:
function testVoteMultipleTimes() public {
orderedCandidates = [candidates[0], candidates[1], candidates[2]];
console.log("Voter: ", voters[0]);
vm.startPrank(voters[0]);
rankedChoice.rankCandidates(orderedCandidates);
address[] memory orderedCandidatesArr = rankedChoice.getUserCurrentVote(
voters[0]
);
console.log("Voted for the following candidates:");
for (uint256 i = 0; i < orderedCandidatesArr.length; i++) {
console.log(orderedCandidatesArr[i]);
}
orderedCandidates = [candidates[3], candidates[4], candidates[5]];
console.log(
"Same voter[0] changed votes for the following candidates:"
);
rankedChoice.rankCandidates(orderedCandidates);
orderedCandidatesArr = rankedChoice.getUserCurrentVote(voters[0]);
for (uint256 i = 0; i < orderedCandidatesArr.length; i++) {
console.log(orderedCandidatesArr[i]);
}
vm.stopPrank();
assertEq(rankedChoice.getUserCurrentVote(voters[0]), orderedCandidates);
}
Output:
Ran 1 test for test/RankedChoiceTest.t.sol:RankedChoiceTest
[PASS] testVoteMultipleTimes() (gas: 498527)
Logs:
Voter: 0x0000000000000000000000000000000000000064
Voted for the following candidates:
0x00000000000000000000000000000000000000C8
0x00000000000000000000000000000000000000C9
0x00000000000000000000000000000000000000ca
Same voter[0] changed votes for the following candidates:
0x00000000000000000000000000000000000000CB
0x00000000000000000000000000000000000000cc
0x00000000000000000000000000000000000000cD
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.07ms (278.97µs CPU time)
Tools Used
Manual Code Review and Foundry
Recommendations
Add a validation within the RanckedChoice::_rankCandidates function to check if the voter has already voted for the same voting cycle, and revert if already voted.
1- In the "RankedChoice.sol" contract add the following storage mapping to track voters and their voting per voting cycle:
+ mapping(address voter => mapping(uint256 voteNumber => bool hasVoted)) public s_votings;
2- Add the following error:
+ error RankedChoice__VoterAlreadyVoted();
3- In the _rankCandidates function, add a condition to check if the voter already voted for the current voting cycle in the beginning of the function:
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;
+ _votings[voter][s_voteNumber] = true;
}
Now, if you rerun the PoC, the "VoterAlreadyVoted" error will be raised if the same voter tried to vote again:
Ran 1 test for test/RankedChoiceTest.t.sol:RankedChoiceTest
[FAIL. Reason: RankedChoice__VoterAlreadyVoted()] testVoteMultipleTimes() (gas: 467995)
Logs:
Voter: 0x0000000000000000000000000000000000000064
Voted for the following candidates:
0x00000000000000000000000000000000000000C8
0x00000000000000000000000000000000000000C9
0x00000000000000000000000000000000000000ca
Same voter[0] changed votes for the following candidates:
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 901.41µs (189.13µs CPU time)
Ran 1 test suite in 3.68ms (901.41µs CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
Failing tests:
Encountered 1 failing test in test/RankedChoiceTest.t.sol:RankedChoiceTest
[FAIL. Reason: RankedChoice__VoterAlreadyVoted()] testVoteMultipleTimes() (gas: 467995)
Encountered a total of 1 failing tests, 0 tests succeeded
