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

Missing ERC721 Receiver Check in executesale function leads to NFT lock via Unsafe Transfer

mentemdeus

Root + Impact

Description

When transferring an ERC721 token, the contract should verify that the recipient is capable of receiving ERC721 tokens. This ensures tokens are not accidentally sent to contracts that cannot handle them and would otherwise become permanently stuck.

In this implementation, the _executeSale function uses BBERC721.transferFrom(address(this), bid.bidder, tokenId) without verifying that bid.bidder is an EOA or an ERC721-compatible contract. As a result, if the bidder is a contract that does not implement onERC721Received, the NFT may be locked and irrecoverable.

function _executeSale(uint256 tokenId) internal {
Listing storage listing = listings[tokenId];
Bid memory bid = bids[tokenId];
​
listing.listed = false;
delete bids[tokenId];
​
BBERC721.transferFrom(address(this), bid.bidder, tokenId); // @> No ERC721 receiver check
​
uint256 fee = (bid.amount * S_FEE_PERCENTAGE) / 100;
s_totalFee += fee;
uint256 sellerProceeds = bid.amount - fee;
_payout(listing.seller, sellerProceeds);
​
emit AuctionSettled(tokenId, bid.bidder, listing.seller, bid.amount);
}

Risk

Likelihood:

This occurs whenever a bidder is a contract that cannot process ERC721 tokens.

Many common smart contracts do not implement IERC721Receiver.

Impact:

NFTs transferred to such contracts become permanently locked.

Users lose access to their purchased assets, reducing trust in the marketplace.

Proof of Concept

The test passes, showing that the NFT was transferred to Bidder without reverting, even though Bidder does not implement onERC721Received. This demonstrates the vulnerability, as the NFT is now in a contract that cannot transfer it out (it’s stuck in the bidder contract).

​
function test_UnsafeNFTTransferToNonReceiver() external {
_mintNFT();
_listNFT();
// Assuming that bidder2 is a contract
//that does not implement onERC721Received.
vm.prank(address(BIDDER_2));
market.placeBid{value: BUY_NOW_PRICE}(tokenId);
​
// Verify the NFT is transferred to the bidder2
assertEq(nft.ownerOf(TOKEN_ID), BIDDER_2, "NFT should be owned by malicious bidder");
​
//Now the nft is stuck in the bidder2 contract
assertEq(nft.balanceOf(BIDDER_2), 1, "Malicious bidder should hold 1 NFT");
}

Recommended Mitigation

Use safeTransferFrom instead of transferFrom to enforce ERC721 receiver compliance:

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

Lead Judging Commences

cryptoghost Lead Judge 2 months 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.

Give us feedback!