The audit of the RankedChoice
smart contract identified a critical vulnerability in the implementation of EIP712 signature verification for off-chain vote submissions.
Location: rankCandidatesBySig
function
Description: The contract attempts to verify EIP712 signatures that include a dynamic array (address[] memory orderedCandidates
). However, EIP712 does not natively support dynamic arrays in the struct type definitions. The current implementation uses abi.encode
with a TYPEHASH
that mismatches the actual data types (uint256[]
vs. address[]
), leading to incorrect hash computation.
Issue Details:
Type Mismatch: The TYPEHASH
specifies uint256[]
, but orderedCandidates
is an address[]
, causing a mismatch in the hashed data.
Dynamic Array Encoding: EIP712 requires special handling for dynamic arrays, typically involving hashing each element and then hashing the array. The contract does not implement this, resulting in incorrect signature hashes.
Potential Exploit: Since signatures will not verify correctly, voters using rankCandidatesBySig
may find their votes unaccepted, leading to a denial of service for off-chain vote submissions.
Denial of Service: Voters attempting to submit their rankings via rankCandidatesBySig
will be unable to do so due to invalid signature verification.
Reduced Participation: The inability to submit votes off-chain can discourage voter participation, undermining the integrity and accessibility of the election process.
Reputation Risk: The malfunctioning of a critical feature can damage the trust in the contract and the organization deploying it.
Correct EIP712 Implementation:
Define a Struct for the Vote: Create a struct that includes all necessary fields, including the voter's address to prevent replay attacks.
Handle Dynamic Arrays Properly: Implement a method to hash the dynamic array according to EIP712 specifications. This often involves hashing each element and then hashing the array as a whole.
Update the TYPEHASH
: Ensure the TYPEHASH
matches the actual data types used in the struct.
Include Voter Address in Signature:
Bind Signature to Voter: Include the voter's address in the signed data to prevent misuse of signatures and enhance security.
Add Nonce for Replay Protection:
Implement Nonce Management: Use a nonce to prevent replay attacks where a signature could be reused to overwrite a voter's ranking unintentionally.
Thorough Testing:
Unit Tests: Implement comprehensive unit tests for the signature verification process.
Integration Tests: Test the off-chain signing and on-chain verification flow end-to-end.
Consult EIP712 Libraries:
Use Established Libraries: Consider using well-tested libraries or utilities that handle EIP712 encoding for dynamic arrays.
TYPEHASH
in rankCandidatesBySig
The TYPEHASH
constant is incorrectly defined as:
bytes32 public constant TYPEHASH = keccak256("rankCandidates(uint256[])");
However, the orderedCandidates
parameter is an array of address
, not uint256
. This mismatch leads to incorrect hash calculations, causing signature verifications to fail or allowing malicious actors to forge signatures.
Recommendation:
Update the TYPEHASH
to match the correct data type:
bytes32 public constant TYPEHASH = keccak256("rankCandidates(address[])");
Ensure that all hashing and signature recovery processes use the correct data types to maintain the integrity of the signature verification.
Severity: High
Description:
The variables s_candidateList
and s_candidateVotesByRound
are not cleared at the beginning of each new election cycle in the selectPresident
function. This oversight can cause residual data from previous elections to interfere with the current election results.
Impact:
Persistent Candidates: Old candidates may remain in s_candidateList
, causing inaccurate candidate pools.
Incorrect Vote Tallies: Previous rounds' votes may persist in s_candidateVotesByRound
, leading to erroneous vote counts.
Recommendation:
Reset s_candidateList
at the start of the selectPresident
function:
Clear s_candidateVotesByRound
for the current s_voteNumber
:
There's no validation to ensure that candidate addresses are valid (e.g., non-zero addresses) or that there are no duplicate candidates in a voter's ranking.
Recommendation:
Validate Non-Zero Addresses:
The getUserCurrentVote
function allows anyone to view a voter's current rankings. If voter anonymity is a concern, this could lead to privacy issues.
Recommendation:
Restrict Access:
Limit the visibility of voters' rankings or remove the function entirely if privacy is essential.
Provide Transparency Notice:
If transparency is acceptable, document this behavior so voters are aware that their rankings are public.
Severity: Informational
Description:
Voters can overwrite their previous rankings by calling rankCandidates
multiple times before the election period ends. While this may be intended, it could be exploited to manipulate the election if not properly managed.
Recommendation:
Prevent Multiple Voting:
If only one vote per voter is desired, add a check:
Accept Vote Changes:
If voters are allowed to change their votes, ensure this behavior is clearly documented.
Functions like _isInArray
and loops within selectPresident
and _selectPresidentRecursive
iterate over dynamic arrays, which could grow large and cause transactions to run out of gas.
Recommendation:
Limit Array Sizes:
Enforce maximum sizes for VOTERS
and candidate arrays.
Optimize Loop Logic:
Consider using mappings or more efficient data structures to minimize gas consumption.
Manual code review
Solidity documentation and EIP712 specification
OpenZeppelin EIP712 utilities
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.