c4-bot-1 commented 8 months ago

Lines of code

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/CultureIndex.sol#L419-L444

Vulnerability details

Impact

The elliptic curve used in Ethereum for signatures is symmetrical, hence for every [v,r,s] there exists another [v,r,s] that returns the same valid result. Therefore two valid signatures exist which allows attackers to compute a valid signature without knowing the signer's private key. ecrecover() is vulnerable to signature malleability 1 2 so it can be dangerous to use it directly.

An attacker can compute another corresponding [v,r,s] that will make this check pass due to the symmetrical nature of the elliptic curve.

Proof of Concept

        function _verifyVoteSignature(
        address from,
        uint256[] calldata pieceIds,
        uint256 deadline,
        uint8 v,
        bytes32 r,
        bytes32 s
    ) internal returns (bool success) {
        require(deadline >= block.timestamp, "Signature expired");

        bytes32 voteHash;

        voteHash = keccak256(abi.encode(VOTE_TYPEHASH, from, pieceIds, nonces[from]++, deadline));

        bytes32 digest = _hashTypedDataV4(voteHash);

        address recoveredAddress = ecrecover(digest, v, r, s);

        // Ensure to address is not 0
        if (from == address(0)) revert ADDRESS_ZERO();

        // Ensure signature is valid
        if (recoveredAddress == address(0) || recoveredAddress != from) revert INVALID_SIGNATURE();

        return true;
    }

Tools Used

manual review

Recommended Mitigation Steps

The easiest way to prevent this issue is to use OpenZeppelin’s ECDSA.sol library and reading the comments above ECDSA's tryRecover() function provides very useful information on correctly implementing signature checks to prevent signature malleability vulnerabilities. More examples: 1 2

When using OpenZeppelin's ECDSA library, special care must be taken to use version 4.7.3 or greater, since previous versions contained a signature malleability bug.