Nouns Streamer is based on Sablier, with some changes. One change that is made is that no transferFrom happens to transfer tokens from the payer to the stream contract. This missing step breaks the entire stream process. The only way that the receiver can receive their tokens is if the payer manually transfers their tokens to the stream contract. This causes the stream to be useless unless the payer performs a manual step to make the stream work.
Vulnerability Detail
Sablier takes a deposit from the payer at the time that a stream is started. This ensures that the stream contract hold a non-zero sum of tokens so that withdrawFromStream can be called by the receiver to withdraw tokens. A key line is checking balance >= amount. Nouns streamer has this same check to make sure the contract balance is greater than the withdraw amount. The problem in Nouns streamer is that no tokens are transferred from the payer to the stream clone when the stream is created. This is a problem because initialize sets remainingBalance to tokenBalance at initialization, but at initialization the stream clone contains no tokens. This means the stream clone holds no tokens and the receiver will not be able to receiver any value unless the sender chooses to manually send the proper number of tokens to the stream clone address, because Nouns streamer does not handle this process automatically.
Impact
Stream process is broken because stream clone contract never receives tokens from sender. When the receiver tries to withdraw from the stream, it will revert unless the sender takes extra manual steps.
davidbrai
commented
1 year ago
keccak123
high
Stream never receives tokens from payer
Summary
Nouns Streamer is based on Sablier, with some changes. One change that is made is that no
transferFromhappens to transfer tokens from the payer to the stream contract. This missing step breaks the entire stream process. The only way that the receiver can receive their tokens is if the payer manually transfers their tokens to the stream contract. This causes the stream to be useless unless the payer performs a manual step to make the stream work.Vulnerability Detail
Sablier takes a deposit from the payer at the time that a stream is started. This ensures that the stream contract hold a non-zero sum of tokens so that
withdrawFromStreamcan be called by the receiver to withdraw tokens. A key line is checkingbalance >= amount. Nouns streamer has this same check to make sure the contract balance is greater than the withdraw amount. The problem in Nouns streamer is that no tokens are transferred from the payer to the stream clone when the stream is created. This is a problem becauseinitializesetsremainingBalancetotokenBalanceat initialization, but at initialization the stream clone contains no tokens. This means the stream clone holds no tokens and the receiver will not be able to receiver any value unless the sender chooses to manually send the proper number of tokens to the stream clone address, because Nouns streamer does not handle this process automatically.Impact
Stream process is broken because stream clone contract never receives tokens from sender. When the receiver tries to withdraw from the stream, it will revert unless the sender takes extra manual steps.
Code Snippet
The key missing line is a
safeTransferFromto send tokens from the payer to the stream clone contract when the stream is created https://github.com/sherlock-audit/2022-11-nounsdao/blob/main/src/StreamFactory.sol#L202-L212Sablier does have this line with
safeTransferFromto take tokens from the sender https://github.com/sablierhq/sablier/blob/476bdd250a4f7ab7fcd79b9d740797da652d0690/packages/protocol/contracts/Sablier.sol#L223Tool used
Manual Review
Recommendation
Add
safeTransferFromto taketokenAmounttokens from the sender on stream creationDuplicate of #3