sherlock-audit / 2022-11-nounsdao-judging

4 stars 0 forks source link

HonorLt - Funding and cancel race condition #42

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

HonorLt

low

Funding and cancel race condition

Summary

If the stream is canceled right before the tokens are sent to the contract, the tokens will be lost.

Vulnerability Detail

Both the payer and recipient can cancel the stream at any time. The contract cannot assume these actions will be coordinated, thus it is possible that the payer is sending a new batch of funding, while the recipient decided to close the stream, and because the recipient's transaction was prioritized (e.g. higher gas price), the stream was closed right before the new tokens arrived. In this case, these newly arrived tokens cannot be retrieved by any party later, because function rescueERC20 only lets the payer withdraw other tokens, meaning the stream tokens will remain in the contract.

EDIT: the stream can be canceled multiple times, so if this situation happens, the payer can retrieve tokens by canceling the stream again if that was the intention. Thus, I am changing the severity to Low.

Impact

While it is not very likely, if that happens, the impact would be huge as streamed tokens were stuck in the contract.

Code Snippet

https://github.com/nounsDAO/streamer/blob/master/src/Stream.sol#L230-L259

https://github.com/nounsDAO/streamer/blob/master/src/Stream.sol#L261-L272

Tool used

Manual Review

Recommendation

When the stream is canceled, a payer should be able to rescue any token:

  if (tokenAddress == address(token()) && remainingBalance != 0) revert CannotRescueStreamToken();