search

sherlock-audit / 2022-11-bullvbear-judging

0 stars 0 forks source link

carrot - Allow changing of recipient for withdrawToken #128

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

carrot

medium

Allow changing of recipient for withdrawToken

Summary

Multi-sig contracts are gaining popularity, and so are smart-contract wallets. However not all smart contract wallets have the necessary callback functions implemented to receive ERC721 tokens. If a user creates a bull position with such a smart-contract wallet, their NFT will be forever stuck in the protocol.

Vulnerability Detail

NFTs can be forever stuck in the protocol if positions are created by a smart contract wallet which cannot receive NFTs / doesnt have the necessary callback function

Impact

NFTs can be forever stuck in the wallet

Code Snippet

https://github.com/sherlock-audit/2022-11-bullvbear/blob/main/bvb-protocol/src/BvbProtocol.sol#L450-L462

Tool used

Manual Review

Recommendation

Can be mitigated in two ways:

  1. Have an admin function with the ability to recover NFTs from withdrawableCollectionTokenId. Users can contact admin to recover NFT to a different EOA/contract wallet
  2. Have a function that lets the recipient nominate a new recipient

Duplicate of #4

datschill commented 1 year ago

PR fixing another issue, removing the withdrawToken() method : https://github.com/BullvBear/bvb-solidity/pull/14