code-423n4 / 2023-12-revolutionprotocol-findings

3 stars 2 forks source link

Protocol targeting to deploy on `Base` and `Optimism` L2s but does not include `chainID` to `domainSeparator` #656

Closed c4-bot-3 closed 10 months ago

c4-bot-3 commented 10 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

A function that uses signature does not include chainId to domainSeparator and can be replayed on Base and Optimism. According to the readme protocol will be deployed to these chains in the feature.

Proof of Concept

In function above chainID not included, although in contract, that out of scope, chainId included to domainSeparator.

Not included:

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

Included:

function castVoteBySig(uint256 proposalId, uint8 support, uint8 v, bytes32 r, bytes32 s) external {
        bytes32 domainSeparator = keccak256(
@>          abi.encode(DOMAIN_TYPEHASH, keccak256(bytes(name)), getChainIdInternal(), address(this))
        );     
        bytes32 structHash = keccak256(abi.encode(BALLOT_TYPEHASH, proposalId, support));
        bytes32 digest = keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash));
        address signatory = ecrecover(digest, v, r, s);
        require(signatory != address(0), "VerbsDAO::castVoteBySig: invalid signature");
        emit VoteCast(signatory, proposalId, support, castVoteInternal(signatory, proposalId, support), "");
    }

Tools Used

vscode

Recommended Mitigation Steps

Include chainId to domainSeparator.

Assessed type

Other

c4-pre-sort commented 10 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 10 months ago

raymondfam marked the issue as duplicate of #6

c4-pre-sort commented 10 months ago

raymondfam marked the issue as duplicate of #354

c4-pre-sort commented 10 months ago

raymondfam marked the issue as duplicate of #267

c4-judge commented 9 months ago

MarioPoneder marked the issue as unsatisfactory: Invalid