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

Missing NFT escrow on listing allows sellers to bypass protocol fees and create permanent ghost listings

Author Revealed upon completion

Description

When a seller calls list(), the NFT should be transferred to the contract so the protocol controls the asset during the sale. This guarantees that buy() can always complete and that the seller cannot sell the NFT through another channel, bypassing protocol fees.

list() never transfers the NFT — it only records the listing in storage. The seller retains full custody and can transfer the NFT away via ERC721 transferFrom at any time. By first canceling and re-listing (to recover collateral at zero cost), the seller can sell the NFT externally, bypass all protocol fees, and leave behind a permanent ghost listing that blocks buyers and the new owner.

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({seller: msg.sender, price: _price, nft: address(this), tokenId: _tokenId, isActive: true});
// NFT never transferred to contract — seller retains full custody
emit NFT_Dealers_Listed(msg.sender, listingsCounter);
}

Risk

Likelihood:

  • Every time a seller lists an NFT, they retain full ERC721 custody and can transfer it away at any time. The protocol has no mechanism to prevent external sales.

  • The collateral recovery path is trivial: list → cancel (get collateral back) → re-list. The second listing costs nothing, so creating ghost listings is free.

Impact:

  • Sellers bypass protocol fees entirely by selling through external channels. The protocol collects zero revenue on these sales.

  • Ghost listings permanently block the tokenId on NFTDealers — buyers who call buy() get reverted, and the new NFT owner cannot list the same token because isActive is still true. The activeListingsCounter is permanently inflated.

Proof of Concept

A seller mints an NFT (20 USDC collateral), lists it at 100 USDC, then immediately cancels — recovering the 20 USDC collateral. The seller re-lists the same token for free (no collateral at risk) and transfers the NFT externally via transferFrom. The seller gets paid on another channel with zero fees to the protocol. The ghost listing stays active permanently: a buyer who tries to buy() gets reverted because the seller no longer owns the NFT, and the new owner is blocked from listing on NFTDealers because isActive is still true.

function test_poc_noEscrowGhostListing() public {
uint256 tokenId = 1;
​
// Seller mints NFT, locking 20 USDC collateral
vm.startPrank(seller);
usdc.approve(address(nftDealers), LOCK_AMOUNT);
nftDealers.mintNft();
vm.stopPrank();
​
// List, then cancel to recover collateral — zero cost from here
vm.prank(seller);
nftDealers.list(tokenId, LISTING_PRICE);
vm.prank(seller);
nftDealers.cancelListing(tokenId);
assertEq(usdc.balanceOf(seller), LOCK_AMOUNT); // collateral recovered
​
// Re-list for free, then transfer NFT externally — bypass fees
vm.prank(seller);
nftDealers.list(tokenId, LISTING_PRICE);
vm.prank(seller);
nftDealers.transferFrom(seller, externalBuyer, tokenId);
​
// Buyer tries to buy the ghost listing — reverts
vm.startPrank(buyer);
usdc.approve(address(nftDealers), LISTING_PRICE);
vm.expectRevert(/* ERC721IncorrectOwner */);
nftDealers.buy(tokenId);
vm.stopPrank();
​
// New owner blocked from listing on NFTDealers
vm.prank(externalBuyer);
vm.expectRevert("NFT is already listed");
nftDealers.list(tokenId, LISTING_PRICE);
​
// Ghost listing is permanent — activeListingsCounter inflated
assertEq(nftDealers.totalActiveListings(), 1);
}

Recommended Mitigation

Escrow the NFT when listing. Transferring the NFT to the contract on list() ensures the seller cannot sell it externally. The protocol controls the asset during the sale, guaranteeing that buy() always succeeds and fees cannot be bypassed.

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({seller: msg.sender, price: _price, nft: address(this), tokenId: _tokenId, isActive: true});
+ _safeTransfer(msg.sender, address(this), _tokenId, "");
emit NFT_Dealers_Listed(msg.sender, listingsCounter);
}

Support

FAQs

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

Give us feedback!