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

Listed NFTs are not escrowed; out-of-band transfer can brick active listings

Author Revealed upon completion

Root + Impact

Description

  • Normal behavior: once an NFT is listed, the marketplace should guarantee executability of buy (by escrowing the NFT or by invalidating the listing when ownership changes).

  • Specific issue: list does not escrow the NFT, so the seller keeps custody and can transfer the token externally. buy later tries _safeTransfer(listing.seller, buyer, tokenId) and reverts because listing.seller is no longer owner. The revert leaves the listing active, and only the original seller can cancel it.

// Root cause in the codebase with @> marks to highlight the relevant section
function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
...
// no NFT escrow/lock is performed here
@> s_listings[_tokenId] =
Listing({seller: msg.sender, price: _price, nft: address(this), tokenId: _tokenId, isActive: true});
}
​
function buy(uint256 _listingId) external payable {
...
@> _safeTransfer(listing.seller, msg.sender, listing.tokenId, "");
@> s_listings[_listingId].isActive = false; // never reached when transfer reverts
}
​
function cancelListing(uint256 _listingId) external {
...
@> require(listing.seller == msg.sender, "Only seller can cancel listing");
}

Risk

Likelihood:

  • This occurs whenever a listed seller transfers the NFT via standard ERC721 transfer paths before a buyer executes buy.

  • Stale listings persist in normal operation because failed buy transactions revert and do not deactivate the listing.

Impact:

  • Buyers face persistent failed purchases for listings shown as active, degrading market reliability and causing denial of service for those listings.

  • Only the original seller can clear the stale listing; when that account is inactive/unresponsive, the listing can remain bricked indefinitely.

Proof of Concept

// 1) seller lists token 1
vm.prank(seller);
nft.list(1, 1_000_000);
​
// 2) seller transfers token 1 out-of-band
address outsider = makeAddr("outsider");
vm.prank(seller);
nft.transferFrom(seller, outsider, 1);
​
// 3) buyer cannot buy because listing.seller is no longer owner
vm.startPrank(buyer);
usdc.approve(address(nft), 1_000_000);
vm.expectRevert();
nft.buy(1);
vm.stopPrank();
​
// 4) listing remains active and outsider cannot cancel
(,,,, bool isActive) = nft.s_listings(1);
assertTrue(isActive);
​
vm.prank(outsider);
vm.expectRevert("Only seller can cancel listing");
nft.cancelListing(1);

Recommended Mitigation

function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
require(ownerOf(_tokenId) == msg.sender, "Not owner of NFT");
...
+ // Escrow NFT in marketplace at listing time
+ _safeTransfer(msg.sender, address(this), _tokenId, "");
s_listings[_tokenId] = Listing(...);
}
​
function buy(uint256 _listingId) external payable {
...
- _safeTransfer(listing.seller, msg.sender, listing.tokenId, "");
+ _safeTransfer(address(this), msg.sender, listing.tokenId, "");
s_listings[_listingId].isActive = false;
}
​
function cancelListing(uint256 _listingId) external {
...
+ // Return escrowed NFT on cancel
+ _safeTransfer(address(this), listing.seller, listing.tokenId, "");
s_listings[_listingId].isActive = false;
}

Support

FAQs

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

Give us feedback!