sherlock-audit / 2022-11-nounsdao-judging

4 stars 0 forks source link

bin2chen - cancel() maybe can't execute #1

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

bin2chen

medium

cancel() maybe can't execute

Summary

if token==usdc and recipient are in the blocklist of usdc, #cancel() will revert, resulting in the inability to get the remaining funds back.

Vulnerability Detail

Cancel() is used when the payer find that the recipient has no right to get the remaining funds, then the payer will call #cancel() to get funds back so that the recipient can't get the subsequent funds (which will increase over time).

But in one case, suppose token==usdc, If the recipient enters the blacklist of usdc for some reason, it will lead to #cancel() revert because usdc.transfer(recipient) will revert, As a result, the payer cannot get the funds back by #cancel(), and no other way to get funds back(#rescueERC20() also can't ) and the funds will always be locked in the contract.

    function cancel() external onlyPayerOrRecipient {
...
        remainingBalance = 0;
        //**@audit if recipient_ in blacklist of usdc , transfer will revert***//
        if (recipientBalance > 0) token_.safeTransfer(recipient_, recipientBalance);

...
    }

usdc contract:

    function transfer(address to, uint256 value)
        external
        override
        whenNotPaused
        notBlacklisted(msg.sender)
        notBlacklisted(to) //*** @audit if in blacklist will revert***//
        returns (bool)
    {
        _transfer(msg.sender, to, value);
        return true;
    }

Impact

payer can't get funds back by #cancel()

Code Snippet

https://github.com/sherlock-audit/2022-11-nounsdao/blob/main/src/Stream.sol#L249

Tool used

Manual Review

Recommendation

use try to Ignore revert

    function cancel() external onlyPayerOrRecipient {
...
        remainingBalance = 0;
-       if (recipientBalance > 0) token_.safeTransfer(recipient_, recipientBalance);
+       if (recipientBalance > 0) {
+            if (tokenBalance() < recipientBalance){
+                revert AmountExceedsBalance();
+            }
+            try token().transfer(recipient_, recipientBalance) {                
+            }catch{} // In rare cases, failure occurs. Even if it really fails, payer keep it 
+        }
...

Duplicate of #37