sherlock-audit / 2022-11-nounsdao-judging

4 stars 0 forks source link

hansfriese - Anyone can fabricate the stream history #64

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

hansfriese

medium

Anyone can fabricate the stream history

Summary

Anyone can create dummy streams, even invalid ones for past.

Vulnerability Detail

While the function createStream() is callable by anyone, it does not check if startTime is greater than block.timestamp. So it is possible for anyone to fabricate the stream history with arbitrary stream data (payer, recipient, tokenAmount, tokenAddress, startTime, stopTime).

Furthermore, there is no way to check if a stream was actually valid one (except the off-chain method to check the withdrawal events but it is not reliable again because a recipient might have not called withdraw yet). If this kind of check is required by any means in the future, it will be difficult to work out.

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

Impact

Anyone can fabricate the stream history and it might affect future usage/tracking.

Code Snippet

https://github.com/sherlock-audit/2022-11-nounsdao/blob/main/src/StreamFactory.sol#L184

Tool used

Manual Review

Recommendation

  1. Consider limiting access to creating streams.
  2. Consider adding a check startTime >= block.timestamp before creating a stream.
  3. Consider adding a flag in the stream to show if actual tokens were deposited/streamed.

Duplicate of #66