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

Active listing can be invalidated by direct NFT transfer

Author Revealed upon completion

Listings do not escrow NFTs. The seller can transfer the token away after listing, leaving the listing active but permanently unbuyable, which breaks marketplace state-machine assumptions.

Description

  • Normal behavior: once an NFT is listed for sale, the listing should remain executable until it is either canceled or purchased.

  • Issue: the protocol does not escrow NFTs when creating a listing. Instead, it only records seller and tokenId in storage.

function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
...
@> s_listings[_tokenId] =
Listing({seller: msg.sender, price: _price, nft: address(this), tokenId: _tokenId, isActive: true});
}

Because the token remains transferable through inherited ERC721 functions, the seller can invalidate the listing by transferring the NFT to another address outside marketplace flow.

Later, buy() attempts to execute transfer using the recorded seller address:

function buy(uint256 _listingId) external payable {
Listing memory listing = s_listings[_listingId];
​
...
@> _safeTransfer(listing.seller, msg.sender, listing.tokenId, "");
}

If the recorded seller no longer owns the NFT, the transfer reverts while the listing remains marked as active.

Risk

Likelihood:

  • Sellers retain full ERC721 transfer rights after listing.

  • Marketplace state does not track ownership changes after listing creation.

Impact:

  • Buyers encounter active listings that cannot be executed, degrading trading reliability.

  • Sellers can grief marketplace flow by creating permanently unbuyable listings.

Proof of Concept

The PoC shows that a seller can list an NFT, transfer it away using the inherited ERC721 interface, and leave the listing active but unexecutable.

function test_ActiveListing_BecomesUnbuyable_AfterDirectNFTTransfer() public revealed {
uint256 tokenId = 1;
uint256 nftPrice = 1000e6;
address attacker2 = makeAddr("hacker1337");
​
mintNFTForTesting();
​
vm.startPrank(userWithCash);
nftDealers.list(tokenId, uint32(nftPrice));
​
nftDealers.transferFrom(userWithCash, attacker2, tokenId);
vm.stopPrank();
​
(,,,, bool isActive) = nftDealers.s_listings(tokenId);
​
assertTrue(isActive);
assertEq(nftDealers.ownerOf(tokenId), attacker2);
​
vm.startPrank(userWithEvenMoreCash);
usdc.approve(address(nftDealers), nftPrice);
​
vm.expectRevert();
nftDealers.buy(tokenId);
vm.stopPrank();
}

PoC rationale:
The test demonstrates that marketplace state assumes listing executability solely based on isActive, while actual token ownership has diverged.

Recommended Mitigation

Listings should either escrow NFTs or enforce ownership validation during lifecycle transitions.

Possible mitigation approaches:

function buy(uint256 _listingId) external payable {
Listing memory listing = s_listings[_listingId];
​
+ require(ownerOf(listing.tokenId) == listing.seller, "Seller no longer owns NFT");

Mitigation rationale:
Ensuring ownership consistency preserves listing executability and prevents marketplace griefing scenarios.

Support

FAQs

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

Give us feedback!