creatorClaimSoldTokens() Does Not Check Destination Address

[code423n4]

Handle

Meta0xNull

Vulnerability details

Impact

function creatorClaimSoldTokens(address destination) public lock {

Surprisingly creatorClaimSoldTokens() Does Not Transfer Token back to streamCreator who is msg.sender but rather Transfer to a New Destination Address.

Is Not Uncommon Users Accidentally Send Tokens into Contract or Zero Address.

ENS Airdrop is a Good Example Users Accidentally Send Tokens into Contract:
https://discuss.ens.domains/t/social-amend-airdrop-proposal-to-include-accidentally-returned-funds/6975

Creator will Lose His HARD EARN MONEY Either Send Token Back into Contract or Send to Zero Address (Creator Only Allowed to Call This Function Once).

Note: Creator Can't Recover His Money via recoverTokens() because It Minus depositTokenAmount That Should Claimed by Creator.

Proof of Concept

https://github.com/code-423n4/2021-11-streaming/blob/main/Streaming/src/Locke.sol#L583-L599
https://github.com/code-423n4/2021-11-streaming/blob/main/Streaming/src/Locke.sol#L654

Tools Used

Manual Review

Recommended Mitigation Steps

1. Simple solution is Transfer Token back to Msg.sender.
   ERC20(depositToken).safeTransfer(msg.sender, amount);

2. If really need to send to destination address, then:
   require(destination != address(0), "Address Can't Be Zero")
   require(destination != address(this), "Address Can't Be This Contract")

[brockelmore]

The stream creator should be considered competent, even more so than a user as they had to create the stream and likely created rewardToken. Additionally, this is likely to be handled in the UI.

[0xean]

Downgrading to non-critical. This is an inherent risk of using a blockchain. A user could send funds to 0x0 as well.