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

Unsafe NFT Transfers Can Lock NFTs in Non-Compliant Contracts

radicalvirtuoso

Root + Impact

Description

  • The ERC721 standard provides safeTransferFrom specifically to prevent NFTs from being locked in contracts that can't handle them.

  • The marketplace ignores this safety mechanism and uses the unsafe transferFrom throughout.

  • The issue is especially severe for auction winners. Smart contracts frequently participate in NFT auctions. When they win, the NFT transfers to them without checking if they can receive ERC721 tokens. If they lack the onERC721Received handler, the NFT becomes permanently inaccessible.

// Root causes in src/BidBeastsNFTMarketPlace.sol
BBERC721.transferFrom(msg.sender, address(this), tokenId); // Line 75
BBERC721.transferFrom(address(this), msg.sender, tokenId); // Line 97
BBERC721.transferFrom(address(this), bid.bidder, tokenId); // Line 213 @> Critical

Risk

Likelihood: Medium

  • Smart contracts often participate in auctions

  • Many contracts lack onERC721Received

  • No validation prevents this

Impact: (Medium)

  • NFT permanently locked

  • No recovery mechanism

  • Affects multisigs, DAOs, trading bots

Proof of Concept

// Deploy a simple contract that can bid but can't receive NFTs
contract IncompatibleBidder {
receive() external payable {}
​
function placeBid(address marketplace, uint256 tokenId) external payable {
BidBeastsNFTMarket(marketplace).placeBid{value: msg.value}(tokenId);
}
// Critical: Missing onERC721Received function
// This contract can send bids but can't properly receive NFTs
}
​
function testNFTLock() public {
// Setup auction with incompatible contract as bidder
IncompatibleBidder badContract = new IncompatibleBidder();
vm.deal(address(badContract), 1 ether);
​
// Contract places winning bid
badContract.placeBid{value: 0.011 ether}(address(marketplace), tokenId);
​
// Auction ends, settlement transfers NFT
vm.warp(block.timestamp + 16 minutes);
marketplace.settleAuction(tokenId);
​
// NFT is now permanently locked - contract owns it but can't transfer it
assertEq(nft.ownerOf(tokenId), address(badContract));
// No way to recover the NFT - it's stuck forever
}

Recommended Mitigation

Line 75:
- BBERC721.transferFrom(msg.sender, address(this), tokenId);
+ BBERC721.safeTransferFrom(msg.sender, address(this), tokenId);
​
Line 97:
- BBERC721.transferFrom(address(this), msg.sender, tokenId);
+ BBERC721.safeTransferFrom(address(this), msg.sender, tokenId);
​
Line 213:
- BBERC721.transferFrom(address(this), bid.bidder, tokenId);
+ BBERC721.safeTransferFrom(address(this), bid.bidder, tokenId);
​
Also add onERC721Received to marketplace:
​
+ function onERC721Received(address, address, uint256, bytes calldata) external returns (bytes4) {
+ require(msg.sender == address(BBERC721), "Only BidBeasts NFTs");
+ return this.onERC721Received.selector;
+ }
Updates

Lead Judging Commences

cryptoghost Lead Judge 21 days 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.

cryptoghost Lead Judge 21 days 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.