sherlock-audit / 2022-11-nounsdao-judging

4 stars 0 forks source link

chainNue - `createStream` didn't check if time is already passed can result instant withdrawal for recipient #71

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

chainNue

medium

createStream didn't check if time is already passed can result instant withdrawal for recipient

Summary

createStream didn't check if time is already passed can result instant payout for recipient (moreover if the creation is using createAndFundStream)

Vulnerability Detail

Ideally the startTime and endTime are in the future time, > block.timestamp. but in createStream() implementation there is no check if startTime and endTime set in the future time. Therefore, if creation of stream is already past startTime & stopTime, this will create this potential vulnerability.

File: StreamFactory.sol
184:     function createStream(
185:         address payer,
186:         address recipient,
187:         uint256 tokenAmount,
188:         address tokenAddress,
189:         uint256 startTime,
190:         uint256 stopTime,
191:         uint8 nonce
192:     ) public returns (address stream) {
193:         // These input checks are here rather than in Stream because these parameters are written
194:         // using clone-with-immutable-args, meaning they are already set when Stream is created and can't be
195:         // verified there. The main benefit of this approach is significant gas savings.
196:         if (payer == address(0)) revert PayerIsAddressZero();
197:         if (recipient == address(0)) revert RecipientIsAddressZero();
198:         if (tokenAmount == 0) revert TokenAmountIsZero();
199:         if (stopTime <= startTime) revert DurationMustBePositive();
200:         if (tokenAmount < stopTime - startTime) revert TokenAmountLessThanDuration();
201: 
202:         stream = streamImplementation.cloneDeterministic(
203:             encodeData(payer, recipient, tokenAmount, tokenAddress, startTime, stopTime),
204:             salt(
205:                 msg.sender, payer, recipient, tokenAmount, tokenAddress, startTime, stopTime, nonce
206:             )
207:         );
208:         IStream(stream).initialize();
209: 
210:         emit StreamCreated(
211:             msg.sender, payer, recipient, tokenAmount, tokenAddress, startTime, stopTime, stream
212:             );
213:     }

according to https://docs.sherlock.xyz/audits/watsons/judging

Medium: There is a viable scenario (even if unlikely) that could cause the protocol to enter a state where a material amount of funds can be lost.

so I think this is categorized as medium issue.

Impact

Instant full payout (withdrawal) for recipient, especially when the creation is using createAndFundStream

Code Snippet

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

Tool used

Manual Review

Recommendation

Add check if startTime and endTime is greater than block.timestamp

Duplicate of #66