keccak123 - createStream can use any address as sender

[sherlock-admin]

keccak123

medium

createStream can use any address as sender

Summary

Nouns Streamer is based on Sablier, with some changes. One change that is made is that any recipient address can be set in createStream. This can lead to token theft of an address that has approved tokens. The payer function argument should never be set by untrusted users.

Vulnerability Detail

There are different createStream functions in StreamFactory with different function arguments. This createStream is different from the others because it allows the user to set any payer address. The other createStream functions use msg.sender as the payer so this value cannot be set to any address. Setting the payer to any address means the stream can be started for another address. The Sablier code that this contract is based on specifically limits payer to msg.sender in this line. If this hardcoding is not done, it is possible for a stream to be started with token flowing out of another address without the address knowledge. This is most problematic in a situation where the other address already has an allowance set. Take this example:

1. Alice starts a stream to Bob
2. Bob decides to start another parallel stream from Alice to Bob to steal most tokens from Alice
3. Alice did not realize Bob created another stream in step 2. Only rescueERC20 could save Alice, and only if she realizes the mistake and if the stream clone has enough tokens

Impact

Hypothetical loss of user funds from lack of access controls on stream start process

Code Snippet

Any payer address can be specified to create a stream https://github.com/sherlock-audit/2022-11-nounsdao/blob/main/src/StreamFactory.sol#L133-L142

The stream is then created without any checks over whether msg.sender is authorized to create a stream on behalf of this payer https://github.com/sherlock-audit/2022-11-nounsdao/blob/main/src/StreamFactory.sol#L184-L213

Stream creation should only be performed by the payer or an authorized party. This is how Sablier handles the process. msg.sender is the stream sender and a deposit is taken from msg.sender at the time the stream is started https://github.com/sablierhq/sablier/blob/476bdd250a4f7ab7fcd79b9d740797da652d0690/packages/protocol/contracts/Sablier.sol#L213 https://github.com/sablierhq/sablier/blob/476bdd250a4f7ab7fcd79b9d740797da652d0690/packages/protocol/contracts/Sablier.sol#L223

Tool used

Manual Review

Recommendation

Remove the ability to set any payer address when creating a stream

[davidbrai]

Streamer doesn't transfer funds from the payer except from one function which allows only msg.sender as the payer. Therefore, I don't see how loss of funds can occur.