Bid Beasts

First Flight #49
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Severity: medium
Valid

Unsafe ERC-721 Transfer Usage Can Permanently Lock NFTs

Root + Impact

Description

  • Normal behavior: When an NFT is sold or transferred, the marketplace should call the ERC-721 transfer functions (e.g. transferFrom or safeTransferFrom) so ownership of the specific tokenId moves from seller (or escrow) to buyer. safeTransferFrom additionally ensures the recipient can receive ERC-721 tokens (via IERC721Receiver) so tokens are not accidentally locked in contracts that don't support ERC-721.

  • Specific issue: The contract erroneously uses an ERC-20 style transfer call (or otherwise treats the token as an ERC-20) when attempting to move an NFT. This either (a) transfers ERC-20 tokens instead of the NFT while leaving the NFT stuck in escrow, or (b) sends the NFT to a non-ERC-721 receiver (for example the address of an ERC-20 token contract) because the wrong interface or destination is used — resulting in the NFT being permanently locked and unrecoverable.

  • Actual behavior in BidBeastsNFTMarket:
    The contract uses transferFrom in both listNFT, unlistNFT, and _executeSale functions:

BBERC721.transferFrom(msg.sender, address(this), tokenId);
...
BBERC721.transferFrom(address(this), msg.sender, tokenId);
...
BBERC721.transferFrom(address(this), bid.bidder, tokenId);

Risk

Likelihood:


High — Many users interact through smart contract wallets, DeFi vaults, or third-party marketplace contracts. If any of these don’t support IERC721Receiver, the NFT is lost during transfer.

Impact:

Medium — Permanent loss of user assets (NFTs) with no recovery mechanism. This can lead to:

  • Irretrievable NFTs stuck in incompatible contracts.

  • Financial losses for sellers/buyers.

  • Severe reputational and legal damage for the marketplace.

Proof of Concept

  • Attacker (or an unsuspecting user) lists an NFT.

  • Auction completes and _executeSale attempts:

BBERC721.transferFrom(address(this), bid.bidder, tokenId);
  • If bid.bidder is a contract that doesn’t implement IERC721Receiver, the NFT is transferred but permanently stuck inside that contract.

  • Neither the marketplace nor the NFT owner can retrieve it.

Recommended Mitigation

Replace all instances of transferFrom with safeTransferFrom:

- BBERC721.transferFrom(address(this), bid.bidder, tokenId);
+ BBERC721.safeTransferFrom(address(this), bid.bidder, tokenId);

and

- BBERC721.transferFrom(msg.sender, address(this), tokenId);
+ BBERC721.safeTransferFrom(msg.sender, address(this), tokenId);

and

- BBERC721.transferFrom(address(this), msg.sender, tokenId);
+ BBERC721.safeTransferFrom(address(this), msg.sender, tokenId);

Unsafe ERC-721 Transfer Usage Can Permanently Lock NFTsThis. This ensures only addresses capable of receiving NFTs (EOAs or ERC-721 compliant contracts) can hold tokens.

Updates

Lead Judging Commences

cryptoghost Lead Judge about 1 month ago
Submission Judgement Published
Validated
Assigned finding tags:

BidBeasts Marketplace: Risk of Locked NFTs

Non-safe transferFrom calls can send NFTs to non-compliant contracts, potentially locking them permanently.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.