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

The lack of sponsor signature authenticity in the `rankCandidatesBySig` function could result in voters incorrectly assuming that the gas fees for the function call are covered by a sponsor

soupy

Summary

Even after correcting the internal function call from _rankCandidates(orderedCandidates, signer) to _rankCandidates(orderedCandidates, msg.sender), the rankCandidatesBySig function still lacks additional signature verification. This means that the function does not validate the authenticity of the input signature, allowing any signature including fake or invalid ones to be used for casting votes. Consequently, voters might end up paying for the gas fees themselves, potentially damaging the credibility and reputation of both the protocol team and the sponsor

https://github.com/Cyfrin/2024-09-president-elector/blob/fccb8e2b6a32404b4664fa001faa334f258b4947/src/RankedChoice.sol#L50-L58

Vulnerability Details

The rankCandidatesBySig function doesn't have secondary signature verification check apart from relying on the check in its internal function _rankCandidates which was found having an input parameter error. Even with the fix of the input parameter error, change from _rankCandidates(orderedCandidates, signer) to _rankCandidates(orderedCandidates, msg.sender), the signature will then be left unchecked as the signature is no longer being verified within the internal function itself.

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);
}

The absence of signature authentic cause the rankCandidatesBySig function takes in any signatures inclusive of invalid signature and yet be able to execute casting voter's vote.

Proof of Concept:

  1. Setup a HelperContract for ease of PoC to interact with various functions in contract RankedChoice and EIP712 during test case.

// SPDX-License-Identifier: MIT
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) {
// it is best to note that the original `TYPEHASH` in the `RankedChoice` contract is also found not appropriately hashed as the function `rankCandidates` takes in an array of address not an array of uint256. Though this step doesn't affect the recovery of signature, for completeness purpose, the following PoC will use the corrected `TYPEHASH` as `keccak256("rankCandidates(address[])")` instead.
bytes32 TYPEHASH = keccak256("rankCandidates(address[])");
bytes32 structHash = keccak256(abi.encode(TYPEHASH, orderedCandidates));
bytes32 hash = _hashTypedDataV4(structHash);
return hash;
}
}

2.In test/RankedChoiceTest.t.sol, add the following test case

function test_audit_signatureAutheticityIssue() public {
orderedCandidates = [candidates[0], candidates[1], candidates[2]];
​
(address fakeSponsor, uint256 fakeKey) = makeAddrAndKey("FakeSponsor");
​
vm.prank(fakeSponsor);
bytes32 hash = helperContract.getAdjustedTypeHashMessage(orderedCandidates);
​
(uint8 v, bytes32 r, bytes32 s) = vm.sign(fakeKey, hash);
bytes memory fakeSignature = abi.encodePacked(r, s, v);
​
// the following steps assume that correction has been done for function `rankCandidatesBySig`: change its internal function from `_rankCandidates(orderedCandidates, signer)` to `_rankCandidates(orderedCandidates, msg.sender)`
address voter = voters_poc[1];
​
vm.startPrank(voter);
helperContract.rankCandidatesBySig(orderedCandidates, fakeSignature);
address[] memory voterVote = helperContract.getUserCurrentVote(voter);
vm.stopPrank();
​
assertEq(voterVote, orderedCandidates);
}

3.Run the test with forge test --match-test test_audit_signatureAutheticityIssue
The test passes showing that even with a fakeSignature, the function can be executed. And since the function is successfully executed fully, it would also mean that voter who make the function call will need to bear with the gas fee since the sponsor signature given is an invalid one.

Impact

Voter who makes the function call with rankCandidatesBySig will need to bear with the gas fee themselves when an invalid sponsor signature is used, leading to unexpected costs for the caller, who might not anticipate that the function is not handling sponsorship correctly.

Tools Used

Manual review with test

Recommendations

To add signature autheticity check on top of the fix with erroneous internal 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);
​
+ if (!_isInArray(VOTERS, signer)) {
+ revert RankedChoice__InvalidSigner();
+ }
​
- _rankCandidates(orderedCandidates, signer);
+ _rankCandidates(orderedCandidates, mgs.sender);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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