Summary
Due to the wrong signer address passed to the internal function _rankCandidates within rankCandidatesBySig, voters who call the function rankCandidatesBySig to cast their votes will not be able to register their votes, making the function rankCandidatesBySig is not usable as intended
https://github.com/Cyfrin/2024-09-president-elector/blob/fccb8e2b6a32404b4664fa001faa334f258b4947/src/RankedChoice.sol#L50-L58
Vulnerability Details
The internal function _rankCandidates in the rankCandidatesBySig was found taking in input parameter using signer address which is the address of the sponsor instead of the voter's address who calls the function to cast their votes. This causes voters won't be able to register their votes using this function.
function rankCandidatesBySig(
address[] memory orderedCandidates,
bytes memory signature
) external {
bytes32 structHash = keccak256(abi.encode(TYPEHASH, orderedCandidates));
bytes32 hash = _hashTypedDataV4(structHash);
address signer = ECDSA.recover(hash, signature);
<@@>! _rankCandidates(orderedCandidates, signer);
}
Proof of Concept:
Setup a HelperContract for ease of PoC to interact with various functions in contract RankedChoice and EIP712 during test case.
pragma solidity 0.8.24;
import {RankedChoice} from "./RankedChoice.sol";
contract HelperContract is RankedChoice {
constructor(address[] memory voters) RankedChoice(voters) {}
function getAdjustedTypeHashMessage(address[] memory orderedCandidates) public view returns(bytes32) {
bytes32 TYPEHASH = keccak256("rankCandidates(address[])");
bytes32 structHash = keccak256(abi.encode(TYPEHASH, orderedCandidates));
bytes32 hash = _hashTypedDataV4(structHash);
return hash;
}
}
2.Modify the test/RankedChoiceTest.t.sol by adding in more specific setup for use in PoC test cases
contract RankedChoiceTest is Test {
address[] voters;
address[] candidates;
address[] voters_poc;
address[] candidates_poc;
address[] orderedCandidates_sponsor = new address[]();
address sponsor;
uint256 sponsorKey;
bytes sponsorSignature;
uint256 constant MAX_VOTERS = 100;
uint256 constant MAX_CANDIDATES = 20;
uint256 constant VOTERS_ADDRESS_MODIFIER = 100;
uint256 constant CANDIDATES_ADDRESS_MODIFIER = 200;
RankedChoice rankedChoice;
HelperContract helperContract;
address[] orderedCandidates;
function setUp() public {
for (uint256 i = 0; i < MAX_VOTERS; i++) {
voters.push(address(uint160(i + VOTERS_ADDRESS_MODIFIER)));
}
rankedChoice = new RankedChoice(voters);
for (uint256 i = 0; i < MAX_CANDIDATES; i++) {
candidates.push(address(uint160(i + CANDIDATES_ADDRESS_MODIFIER)));
}
address[] memory newVoters = new address[]();
(sponsor, sponsorKey) = makeAddrAndKey("Sponsor");
newVoters[0] = sponsor;
for (uint256 i = 0; i < voters.length; i++) {
newVoters[i + 1] = voters[i];
}
voters_poc = newVoters;
for (uint256 i = 0; i < 10; i++) {
candidates_poc.push(address(uint160(i + CANDIDATES_ADDRESS_MODIFIER)));
}
orderedCandidates_sponsor[0] = candidates_poc[0];
orderedCandidates_sponsor[1] = candidates_poc[1];
orderedCandidates_sponsor[2] = candidates_poc[2];
helperContract = new HelperContract(voters_poc);
}
modifier tapoutSponsorSignature() {
vm.prank(sponsor);
bytes32 hash = helperContract.getAdjustedTypeHashMessage(orderedCandidates_sponsor);
(uint8 v, bytes32 r, bytes32 s) = vm.sign(sponsorKey, hash);
bytes memory signature = abi.encodePacked(r, s, v);
sponsorSignature = signature;
_;
}
function test_audit_wrongSignerAddressUsedInRankCandidatesBySig() public tapoutSponsorSignature {
address voter = voters_poc[1];
vm.startPrank(voter);
helperContract.rankCandidatesBySig(orderedCandidates_sponsor, sponsorSignature);
address[] memory voterVote = helperContract.getUserCurrentVote(voter);
vm.stopPrank();
assertEq(voterVote, orderedCandidates_sponsor);
}
3.Run the test with forge test --match-test test_audit_wrongSignerAddressUsedInRankCandidatesBySig -vv
The test will show assertion failed with voter's orderedCandidates list is empty, which proves that voters can't cast their votes when interacting with rankCandidatesBySig.
Console log as follow:
$ forge test --match-test test_audit_wrongSignerAddressUsedInRankCandidatesBySig -vv [15:15:26]
[⠊] Compiling...
[⠰] Compiling 1 files with Solc 0.8.24
[⠔] Solc 0.8.24 finished in 1.32s
Compiler run successful!
Ran 1 test for test/RankedChoiceTest.t.sol:RankedChoiceTest
[FAIL. Reason: assertion failed: [] != [0x00000000000000000000000000000000000000C8, 0x00000000000000000000000000000000000000C9, 0x00000000000000000000000000000000000000ca]] test_audit_wrongSignerAddressUsedInRankCandidatesBySig() (gas: 453853)
Logs:
voter's orderedCandidate length: 0
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 7.11ms (605.50µs CPU time)
Impact
Voters won't be able to cast their votes using rankCandidatesBySig. The function is not usable as intended.
Tools Used
Manual review with test
Recommendations
To change the input parameter used in the internal function from signer to msg.sender, so the caller's (which is the voter) orderedCandidate list get registered as their own vote when using a sponsored signature to cast the votes.
function rankCandidatesBySig(
address[] memory orderedCandidates,
bytes memory signature
) external {
bytes32 structHash = keccak256(abi.encode(TYPEHASH, orderedCandidates));
bytes32 hash = _hashTypedDataV4(structHash);
address signer = ECDSA.recover(hash, signature);
- _rankCandidates(orderedCandidates, signer);
+ _rankCandidates(orderedCandidates, mgs.sender);
}
