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

Unsafe NFT transfer using `transferFrom` instead of `safeTransferFrom`, can lead to NFT being locked in a contract that is not well prepared to handle NFTs.

ntdegenerate

Root + Impact

Unsafe NFT transfer using transferFrom instead of safeTransferFrom, can lead to NFT being locked in a contract that is not well prepared to handle NFTs.

Description

When an auction is to be settled, and the NFT sale is to be executed using BidBeastsNFTMarket:_executeSale, the NFT tranfer is done using transferFrom. While this is fine for EOA recievers, Smart contract recievers are at a risk of locking the NFT forever if they don't have mechanism in place to transfer the NFT out of the contract.

With transferFrom, the implementation of onERC721Received is not enforced, meaning that any Smart contract will be able to receive the NFT transfer, whether it is prepared to handle NFTs or not.

https://github.com/CodeHawks-Contests/2025-09-bid-beasts/blob/449341c55a57d3f078d1250051a7b34625d3aa04/src/BidBeastsNFTMarketPlace.sol#L213

Risk

Likelihood:

  • Reason 1: This happens anytime an auction sale is executed

Impact:

  • Potential locking of received NFT forever

Proof of Concept

This shows how a smart contract receiver that does not have the standard IERC721Receiver implemented receives an NFT, with no way to transfer it out of the contract, essentially locking the NFT in the contract.
Place the following code in BidBeastsNFTMarketTest.t.sol

function testNonSafeTransfer() public warmMarket {
address naiveBuyer = makeAddr("naiveBuyer");
vm.deal(naiveBuyer, 10 ether);
vm.prank(BIDDER_1);
market.placeBid{value: MIN_PRICE + 0.5 ether}(TOKEN_ID);
​
vm.startPrank(naiveBuyer);
NaiveReceiver receiver = new NaiveReceiver{value: 10 ether}(address(market));
receiver.buyNftNow(TOKEN_ID);
vm.stopPrank();
assertEq(nft.ownerOf(TOKEN_ID), address(receiver));
assertEq(address(receiver).balance, 5 ether);
}
contract NaiveReceiver {
BidBeastsNFTMarket market;
​
constructor(address _market) payable {
market = BidBeastsNFTMarket(_market);
}
​
function buyNftNow(uint256 tokenId) public {
market.placeBid{value: market.getListing(tokenId).buyNowPrice + 1 ether}(tokenId);
}
​
receive() external payable {}
}
​

Recommended Mitigation

Use the standard ERC721 safeTransferFrom function to transfer NFTs instead of transferFrom

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