sherlock-audit / 2022-11-nounsdao-judging

4 stars 0 forks source link

ctf_sec - The caller can set start and stop timestamp far away from the current timestamp to let recipient never receive the token or set start and stop timestamp to past timestamp to game the recipient #32

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ctf_sec

medium

The caller can set start and stop timestamp far away from the current timestamp to let recipient never receive the token or set start and stop timestamp to past timestamp to game the recipient

Summary

The caller can set start and stop timestamp far away from the current timestamp to let recipient never receive the token or set start and stop timestamp to past timestamp to game the recipient

Vulnerability Detail

When creating the stream, the code validating if the stop time is greater than start time,

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

note the code:

if (stopTime <= startTime) revert DurationMustBePositive();

however, the code does not verify the start time timestamp and past timestamp is before block.timestamp. and does not if the start time and stop time is too far away from the current timestamp.

Impact

If The caller can set start and stop timestamp far away from the current timestamp, for example, set the start time to 200 years later and stop time to 300 years later, the recipient may never has the chance to receive the payment.

Or the caller can set the start timestamp to the last or the stop timestamp to the past.

Code Snippet

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

Tool used

Manual Review

Recommendation

We recommend the project not let user set the timestamp in the past and not let stop and end timestamp too far away from the current block.timestamp

Duplicate of #66