Payer’s funds might be permanently locked in certain cases
Summary
In some cases it might be impossible for the payer to cancel the stream and retrieve the funds which were not yet entitled to the receiver. Due to the same root cause, it might be impossible for the payer to withdraw any amount of tokens which were sent to the stream contract on top of the hardcoded tokenAmount.
Vulnerability Detail
The only way for the payer to retrieve the stream specific funds in the contract is through the cancel() function (any other non-stream specific funds can be retrieved using rescueERC20()). There are two cases in which the payer would like to retrieve his funds:
In case he cancels the stream before the stream has ended. All the funds which were not yet entitled to the recipient will still belong to the payer.
In case, by mistake, the payer sent additional stream specific tokens on top of the hardcoded tokenAmount and would like to withdraw those from the contract.
In both cases the payer has to cancel the stream using cancel(). However, if the recipient is still entitled to some of the funds in the stream (as he/she did not withdraw them yet), they will be withdrawn in the following line:
According to the docs, Streams support the USDC token. However, USDC supports blacklisting, which could block certain addresses from both sending and receiving USDC:
Therefore, if a recipient gets blacklisted on USDC (e.g. by interacting with Tornado Cash on purpose), the cancel() function will revert and the payer is unable to retrieve the funds he’s/she’s entitled to upon cancellation, or any extra stream specific funds which were sent to the contract.
Impact
If a stream gets funded after the stream is created, there is no way to block any funds on top of the hardcoded tokenAmount. Therefore, if the payer accidentally sends too much tokens, a recipient might act maliciously and perform certain actions to be placed on the USDC blacklist.
Let’s suppose this happens just after the stream started, when the recipient is entitled to a few wei. However, the few wei lost might not be an issue for the recipient compared to amount of damage the recipient could cause by getting blacklisted. In that case, a potentially large amount of USDC which was sent by the funder will be permanently locked in the contract. Hence, we deem the risk as high.
Although it will lead to additional gas costs, it is recommended in the cancel() function to keep track of the funds to which the payer and recipient are entitled to whenever one of both cancel’s the stream. An additional function can be created to subsequently withdraw the funds of either the payer or recipient. This way the token transfer to the payer will not be impacted if the token transfer to the recipient reverts.
Zarf
high
Payer’s funds might be permanently locked in certain cases
Summary
In some cases it might be impossible for the payer to cancel the stream and retrieve the funds which were not yet entitled to the receiver. Due to the same root cause, it might be impossible for the payer to withdraw any amount of tokens which were sent to the stream contract on top of the hardcoded
tokenAmount.Vulnerability Detail
The only way for the payer to retrieve the stream specific funds in the contract is through the
cancel()function (any other non-stream specific funds can be retrieved usingrescueERC20()). There are two cases in which the payer would like to retrieve his funds:tokenAmountand would like to withdraw those from the contract.In both cases the payer has to cancel the stream using
cancel(). However, if the recipient is still entitled to some of the funds in the stream (as he/she did not withdraw them yet), they will be withdrawn in the following line:https://github.com/sherlock-audit/2022-11-nounsdao/blob/main/src/Stream.sol#L249
According to the docs, Streams support the USDC token. However, USDC supports blacklisting, which could block certain addresses from both sending and receiving USDC:
https://github.com/centrehq/centre-tokens/blob/0d3cab14ebd133a83fc834dbd48d0468bdf0b391/contracts/v1/FiatTokenV1.sol#L272-L282
Therefore, if a recipient gets blacklisted on USDC (e.g. by interacting with Tornado Cash on purpose), the
cancel()function will revert and the payer is unable to retrieve the funds he’s/she’s entitled to upon cancellation, or any extra stream specific funds which were sent to the contract.Impact
If a stream gets funded after the stream is created, there is no way to block any funds on top of the hardcoded
tokenAmount. Therefore, if the payer accidentally sends too much tokens, a recipient might act maliciously and perform certain actions to be placed on the USDC blacklist.Let’s suppose this happens just after the stream started, when the recipient is entitled to a few wei. However, the few wei lost might not be an issue for the recipient compared to amount of damage the recipient could cause by getting blacklisted. In that case, a potentially large amount of USDC which was sent by the funder will be permanently locked in the contract. Hence, we deem the risk as high.
Code Snippet
https://github.com/sherlock-audit/2022-11-nounsdao/blob/main/src/Stream.sol#L237-L259
Tool used
Manual Review
Recommendation
Although it will lead to additional gas costs, it is recommended in the
cancel()function to keep track of the funds to which the payer and recipient are entitled to whenever one of both cancel’s the stream. An additional function can be created to subsequently withdraw the funds of either the payer or recipient. This way the token transfer to the payer will not be impacted if the token transfer to the recipient reverts.Duplicate of #37