0xSmartContract - Cross-chain replay attacks are possible with `createStream()`

[sherlock-admin]

0xSmartContract

medium

Cross-chain replay attacks are possible with createStream()

Summary

StreamFactory.sol#L202

Mistakes made on one chain can be re-applied to a new chain

There is no chain.id in the createStream() function data

stream = streamImplementation.cloneDeterministic(
            encodeData(payer, recipient, tokenAmount, tokenAddress, startTime, stopTime),
            salt(
                msg.sender, payer, recipient, tokenAmount, tokenAddress, startTime, stopTime, nonce
            )

Vulnerability Detail

Mistakes made on one chain can be re-applied to a new chain

There is no chain.id in the createStream() function data

Impact

If a user does createStream() using the wrong network, an attacker can replay the action on the correct chain, and steal the funds a-la the wintermute gnosis safe attack, where the attacker can create the same address that the user tried to, and steal the funds from there

https://mirror.xyz/0xbuidlerdao.eth/lOE5VN-BHI0olGOXe27F0auviIuoSlnou_9t3XRJseY

Code Snippet

StreamFactory.sol#L202

function createStream(
        address payer,
        address recipient,
        uint256 tokenAmount,
        address tokenAddress,
        uint256 startTime,
        uint256 stopTime,
        uint8 nonce
    ) public returns (address stream) {
        // These input checks are here rather than in Stream because these parameters are written
        // using clone-with-immutable-args, meaning they are already set when Stream is created and can't be
        // verified there. The main benefit of this approach is significant gas savings.
        if (payer == address(0)) revert PayerIsAddressZero();
        if (recipient == address(0)) revert RecipientIsAddressZero();
        if (tokenAmount == 0) revert TokenAmountIsZero();
        if (stopTime <= startTime) revert DurationMustBePositive();
        if (tokenAmount < stopTime - startTime) revert TokenAmountLessThanDuration();

        stream = streamImplementation.cloneDeterministic(
            encodeData(payer, recipient, tokenAmount, tokenAddress, startTime, stopTime),
            salt(
                msg.sender, payer, recipient, tokenAmount, tokenAddress, startTime, stopTime, nonce
            )
        );
        IStream(stream).initialize();

        emit StreamCreated(
            msg.sender, payer, recipient, tokenAmount, tokenAddress, startTime, stopTime, stream
            );
    }

Tool used

Manual Review

Recommendation

Include the chain.id in createStream() function

Replay attacks usually come into play when a chain undergoes a hard fork and hence we start hearing a bit more about them when such an event is on the horizon.

During the first Ethereum split, between ETH and ETH Classic, this became an issue as ETH classic did not have replay protection built in. However, the ETH community resolved this by introducing a field called ChainID which is unique to each EVM-based chain.

The ETHW community has decided to use the ChainID 10001. Thereby automatically protecting the chains against replay attacks.

[davidbrai]

I'm trying to understand the attack scenario here. what I got so far:

User calls createStream on chain A but sends funds to the stream on chain B by mistake? then an attacker can deploy createStream on chain B? I think for that to work the StreamFactory needs to be deployed on chain B with the same address. We currently only plan to deploy to ethereum. And even if StreamFactory is deployed on chain B, even if there was replay protection, the funds would still be lost.

So I'm not exactly sure how this replay protection helps.