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

Use of transferFrom instead of safeTransferFrom for NFT transfers

0xon3_7zr8

Impact:
Medium — Direct NFT transfers to contracts that do not implement ERC721Receiver can fail silently, causing NFTs to be locked in the marketplace contract. This affects listings, unlisting, and auction settlements.

Likelihood:
Medium — Any seller or winner sending NFTs to a smart contract rather than an EOA can trigger this issue.

Scope (affected files):

  • src/BidBeastsNFTMarket.sol

    • listNFT()

    • unlistNFT()

    • _executeSale()

Description (Root + Impact)

Normal behaviour:
NFTs should always be transferred safely, whether to sellers or auction winners, including when the recipient is a smart contract.

Issue:
The contract uses transferFrom for NFT transfers in multiple places, which does not check if the recipient contract can handle ERC721 tokens. NFTs sent to incompatible contracts may get irreversibly locked.

Root cause:

// unlistNFT()
@> BBERC721.transferFrom(address(this), msg.sender, tokenId); // should be safeTransferFrom
​
// listNFT() – NFT received from user (OK to transferFrom from EOA, but consider safeTransferFrom for general safety)
// _executeSale()
@> BBERC721.transferFrom(address(this), bid.bidder, tokenId); // should be safeTransferFrom
​

Why this matters:

  • NFTs sent to contracts without ERC721 receiver support cannot be recovered.

  • Marketplace UX suffers, and users may permanently lose access to their tokens.

Risk

Reason 1: Any unlisting or auction settlement to a contract address can lock NFTs.
Reason 2: Users unaware of ERC721Receiver limitations cannot retrieve locked assets.

Impact:

  • Permanent NFT loss for users.

  • Trust and reputation risk for the platform.

Recommended Mitigation

- BBERC721.transferFrom(address(this), msg.sender, tokenId);
+ BBERC721.safeTransferFrom(address(this), msg.sender, tokenId);
​
- BBERC721.transferFrom(address(this), bid.bidder, tokenId);
+ BBERC721.safeTransferFrom(address(this), bid.bidder, tokenId);
​
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.