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

3 stars 2 forks source link

Risk of cross-chain signature replay attack #279

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#L375 https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/CultureIndex.sol#L404 https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/CultureIndex.sol#L419

Vulnerability details

Impact

The calculated signature voteHash lacks the chainId parameter, which opens the signature to a possibility of a cross-chain replay attack.

Proof of Concept

The voteForManyWithSig and batchVoteForManyWithSig allows for onbehalf voting for other users using signatures. To check for validity of the signature, the _verifyVoteSignature function is called which encodes the signature's params into a voteHash

    function _verifyVoteSignature(
        address from,
        uint256[] calldata pieceIds,
        uint256 deadline,
        uint8 v,
        bytes32 r,
        bytes32 s
    ) internal returns (bool success) {
        ...
        bytes32 voteHash;

        voteHash = keccak256(abi.encode(VOTE_TYPEHASH, from, pieceIds, nonces[from]++, deadline)); //@note
        ...
        return true;
    }

Considering that the protocol is expected to be deployed on mainnet and optimism stacks, and that each user is expected to vote just once, the lack of a chainid in the voteHash allows for the signature to be replayed on all the available chains.

Tools Used

Manual code review

Recommended Mitigation Steps

As specified by the EIP4337 standard to prevent replay attacks, the signature should depend on chainid., so consider adding the chainId param to the voteHash

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 10 months ago

MarioPoneder marked the issue as unsatisfactory: Invalid