search

code-423n4 / 2021-11-streaming-findings

0 stars 0 forks source link

creatorClaimSoldTokens() Does Not Check Destination Address #77

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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