IPCTDelegate Manual Review Findings

IPT-01M: Insecure Elliptic Curve Recovery Mechanism

Description:

The ecrecover function is a low-level cryptographic function that should be utilized after appropriate sanitizations have been enforced on its arguments, namely on the s and v values. This is due to the inherent trait of the curve to be symmetrical on the x-axis and thus permitting signatures to be replayed with the same x value (r) but a different y value (s).

Example:

Copy
400/**
401 * @notice Cast a vote for a proposal by signature
402 * @dev External function that accepts EIP-712 signatures for voting on proposals.
403 */
404function castVoteBySig(
405    uint256 proposalId,
406    uint8 support,
407    uint8 v,
408    bytes32 r,
409    bytes32 s
410) external {
411    bytes32 domainSeparator = keccak256(
412        abi.encode(DOMAIN_TYPEHASH, keccak256(bytes(NAME)), getChainIdInternal(), address(this))
413    );
414    bytes32 structHash = keccak256(abi.encode(BALLOT_TYPEHASH, proposalId, support));
415    bytes32 digest = keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash));
416    address signatory = ecrecover(digest, v, r, s);
417    require(signatory != address(0), "IPCT::castVoteBySig: invalid signature");
418    emit VoteCast(
419        signatory,
420        proposalId,
421        support,
422        castVoteInternal(signatory, proposalId, support),
423        ""
424    );
425}

Recommendation:

We advise them to be sanitized by ensuring that v is equal to either 27 or 28 (v ∈ {27, 28}) and to ensure that s is existent in the lower half order of the elliptic curve (0 < s < secp256k1n ÷ 2 + 1) by ensuring it is less than 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A1. A reference implementation of those checks can be observed in the ECDSA library of OpenZeppelin and the rationale behind those restrictions exists within Appendix F of the Yellow Paper.

Alleviation:

The cryptographic sanitisations we advised have been properly integrated in the codebase.