NFT Dealers
First Flight #58
Beginner FriendlyFoundry
100 EXP
View all submissions
Previous Next
Submission Details
Impact: high
Likelihood: medium

[H-2] NFT Not Escrowed at Listing Time - Seller Can Grief Buyers

Author Revealed upon completion

Root + Impact

Description

  • When a seller lists an NFT, the marketplace is expected to hold the NFT (or at minimum ensure the seller cannot transfer it) until the listing is resolved. This prevents the seller from listing at an attractive price and then removing the NFT.

  • list() does not transfer the NFT to the contract or revoke the seller's transfer ability. The seller retains full ownership and can transfer the NFT to another address at any time while the listing stays active. A buyer calling buy() will have their USDC transferred successfully, after which _safeTransfer reverts because the seller no longer holds the token, causing the full transaction to revert. However, the listing remains permanently active, and the seller can repeat this griefing cycle indefinitely.

// src/NFTDealers.sol
function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
require(_price >= MIN_PRICE, "Price must be at least 1 USDC");
require(ownerOf(_tokenId) == msg.sender, "Not owner of NFT");
require(s_listings[_tokenId].isActive == false, "NFT is already listed");
​
@> // NFT is never transferred to the contract or locked here
s_listings[_tokenId] = Listing({seller: msg.sender, price: _price, nft: address(this), tokenId: _tokenId, isActive: true});
emit NFT_Dealers_Listed(msg.sender, listingsCounter);
}

Risk

Likelihood:

  • A seller transferring their NFT after listing is a single additional transaction with no cost beyond gas.

  • The listing remains permanently active with no automatic deactivation mechanism.

Impact:

  • Buyers waste gas on repeated failed purchase attempts against griefed listings.

  • Sellers can use a single active listing as a denial-of-service tool against a specific token.

Proof of Concept

A seller lists an NFT, then immediately transfers it to a dead address. The listing remains active. Every buy attempt by a legitimate buyer reverts, but the listing cannot be cleared.

function testSellerGriefsBuyer() public revealed {
mintAndListNFTForTesting(1, 1000e6);
​
// Seller transfers NFT away after listing
vm.prank(userWithCash);
nftDealers.transferFrom(userWithCash, address(0xdead), 1);
​
// Listing is still active
(,,,, bool isActive) = nftDealers.s_listings(1);
assertTrue(isActive);
​
// Buyer attempts purchase — reverts after USDC is pulled (whole tx reverts)
vm.startPrank(userWithEvenMoreCash);
usdc.approve(address(nftDealers), 1000e6);
vm.expectRevert(); // ERC721 transfer from incorrect owner
nftDealers.buy(1);
vm.stopPrank();
}

Recommended Mitigation

Transfer the NFT into the contract at listing time and return it to the seller on cancellation. This guarantees atomicity between listing and sale.

function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
require(_price >= MIN_PRICE, "Price must be at least 1 USDC");
require(ownerOf(_tokenId) == msg.sender, "Not owner of NFT");
require(s_listings[_tokenId].isActive == false, "NFT is already listed");
​
listingsCounter++;
activeListingsCounter++;
​
s_listings[_tokenId] = Listing({...});
+ _transfer(msg.sender, address(this), _tokenId); // escrow NFT in contract
emit NFT_Dealers_Listed(msg.sender, listingsCounter);
}
​
function cancelListing(uint256 _listingId) external {
...
s_listings[_listingId].isActive = false;
activeListingsCounter--;
​
+ _transfer(address(this), listing.seller, listing.tokenId); // return NFT to seller
usdc.safeTransfer(listing.seller, collateralForMinting[listing.tokenId]);
collateralForMinting[listing.tokenId] = 0;
...
}

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!