NFT Dealers

Summary

When a buyer purchases an NFT and re-lists it before the original seller calls collectUsdcFromSelling, the list() function fully overwrites s_listings[tokenId] — destroying the original seller's data. Since collectUsdcFromSelling uses the onlySeller modifier which checks s_listings[tokenId].seller, the original seller is permanently blocked from collecting their sale proceeds and collateral. Their funds are locked in the contract forever with no recovery mechanism.

Impact

Any seller who does not immediately call collectUsdcFromSelling after their NFT is sold risks permanent loss of their funds. The buyer has every right to re-list the token they now own, and the protocol imposes no time limit or urgency on the seller to collect. Yet a single re-list by the buyer destroys the seller's access to money the protocol is holding on their behalf.

Consider Alice, who mints an NFT ($20 collateral) and lists it for $1,000 USDC. Bob buys it. Alice plans to collect her proceeds later that day, but Bob re-lists the token within minutes. Alice's listing entry is overwritten — seller is now Bob, not Alice. When Alice calls collectUsdcFromSelling, the onlySeller modifier reverts because the contract no longer recognizes her as the seller. Alice's $1,000 sale proceeds and $20 collateral are permanently locked in the contract. There is no admin function, no alternative path, and no way to recover the funds.

Critically, this issue cascades across every resale of the same token. Once the token is re-listed, the original seller is blocked by two independent locks: (1) onlySeller fails because the seller field now points to the new lister, and (2) require(!listing.isActive) fails because the new listing is active. If the pattern repeats — Bob sells to Carol, Carol re-lists before Bob collects — Bob's proceeds are also permanently locked. Every seller in the chain loses their funds if the next buyer re-lists before they collect. USDC accumulates in the contract with no one able to claim it. The buyer does not need to act maliciously — simply using the protocol normally (buying and re-listing) triggers this loss. There is no event, no warning, and no time limit indicating that collection is urgent.

Vulnerability Details

The root cause is that list() uses _tokenId as the mapping key and performs a full overwrite with no check for uncollected proceeds from a prior sale:

After buy() executes, the listing becomes inactive but the original seller's data persists:

When the buyer calls list() with the same token ID, all three require checks pass:

• ownerOf(_tokenId) == msg.sender — buyer owns it now

• s_listings[_tokenId].isActive == false — buy() set it to false

• Price checks pass

The full overwrite on line 140-141 replaces the seller address with the new lister. The original seller's collectUsdcFromSelling call then fails at the onlySeller modifier:

Since s_listings[tokenId].seller is now the buyer (new lister), the original seller can never pass this check.

Code Location

• src/NFTDealers.sol:140-141 — s_listings[_tokenId] full overwrite with no check for uncollected proceeds

• src/NFTDealers.sol:134 — isActive == false check passes after buy(), enabling re-list

• src/NFTDealers.sol:76-79 — onlySeller modifier blocks original seller after overwrite

• src/NFTDealers.sol:156 — buy() sets isActive = false but preserves seller data (correct, but unprotected from overwrite)

Proof of Concept

POC Guide

1. Copy the test code below

2. Paste it into test/NFTDealersTest.t.sol

3. Run: forge test --mt test_relistOverwritesPendingProceeds_POC -vvv

Test Code

Location: test/NFTDealersTest.t.sol

Output

Mitigation

Add a check in list() to prevent re-listing a token that has uncollected proceeds from a prior sale. The simplest approach is to verify that collateralForMinting[_tokenId] has been zeroed (indicating the previous seller has collected or cancelled):

Alternatively, use listingsCounter as the mapping key instead of _tokenId, giving each listing a unique ID that is never overwritten. This would require updating all functions that reference s_listings to use listing IDs instead of token IDs.