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

NFT can be locked if the winning bidder is a smart contract

duma999

NFT can be locked if the winning bidder is a smart contract

Description

The function _executeSale transfers the nft to the winning bidder with the function transferFrom, but this function does not check if the receiver can indeed receive the nft.

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);
​
...
}
​

Risk

impact : The impact is that the token can be locked forever in a contract that is not ERC721 compatible. (medium)

Proof of Concept

But I have made some tests with an empty contract call RejectEther with the following tests to add to the test file:

contract RejectEther {
// Intentionally has no payable receive or fallback
}
​
...
​
function test_Fail_NFT_transfer() public {
_mintNFT();
_listNFT();
​
vm.prank(address(rejector));
​
market.placeBid{value: BUY_NOW_PRICE}(TOKEN_ID);
​
assertEq(nft.balanceOf(address(market)), 0, "Failed nft transfert");
assertEq(nft.balanceOf(address(rejector)), 1, "Failed nft transfert");
assertEq(nft.ownerOf(TOKEN_ID), address(rejector), "Failed nft transfert");
​
// Can transfer his nft even though the rejector contract does not have the oNReceiver721 function
vm.prank(address(rejector));
nft.transferFrom(address(rejector), BIDDER_1, TOKEN_ID);
assertEq(nft.balanceOf(address(rejector)), 0, "Failed nft transfert");
assertEq(nft.balanceOf(BIDDER_1), 1, "Failed nft transfert");
}

The test work, the empty contract does not have any problem to transfer his nft.

Recommended Mitigation

Nevertheless, the standard approach is to use the safeTransferFrom function. So that it checks if the receiver is a contract and if it has implemented the onReceiver function.

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.

Appeal created

duma999 Submitter
2 months ago
cryptoghost Lead Judge
2 months ago
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!