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

Listing ID Collision Overwrites Unclaimed Seller Proceeds

Author Revealed upon completion

Root + Impact

Description

  • The normal behavior is that each listing has its own immutable sale record so a seller can always settle their completed sale later, even after future relistings of the same NFT.

  • The issue is that listings are stored by tokenId (s_listings[_tokenId]) instead of by a unique listing id, while the contract still increments listingsCounter. When the same NFT is relisted, the old listing record is overwritten, which can block or misdirect settlement for earlier sales.

function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
...
listingsCounter++;
activeListingsCounter++;
​
s_listings[_tokenId] = // @> keyed by tokenId, not by unique listingsCounter
Listing({seller: msg.sender, price: _price, nft: address(this), tokenId: _tokenId, isActive: true});
emit NFT_Dealers_Listed(msg.sender, listingsCounter); // @> emitted id does not match storage key
}
​
modifier onlySeller(uint256 _listingId) {
require(s_listings[_listingId].seller == msg.sender, "Only seller can call this function");
_;
}
​
function collectUsdcFromSelling(uint256 _listingId) external onlySeller(_listingId) {
Listing memory listing = s_listings[_listingId]; // @> reads latest overwritten record for that tokenId slot
...
}

Risk

Likelihood:

  • This occurs whenever a sold NFT is relisted before the previous seller calls settlement.

  • This occurs naturally in active markets where assets are bought and relisted quickly.

Impact:

  • Previous seller can lose access to proceeds because onlySeller now points to the new seller in the overwritten slot.

  • Settlement/accounting correctness breaks, creating stranded funds and potential theft/griefing conditions.

Proof of Concept

// 1) Alice mints token #1 and lists it.
// 2) Bob buys token #1. Alice has not called collectUsdcFromSelling yet.
// 3) Bob relists token #1.
// -> s_listings[1] is overwritten with seller=Bob.
// 4) Alice calls collectUsdcFromSelling(1).
// -> reverts in onlySeller because seller is now Bob.
// 5) Alice's proceeds from Bob's purchase are now inaccessible via the original listing record.

Recommended Mitigation

- mapping(uint256 => Listing) public s_listings;
+ mapping(uint256 => Listing) public s_listings;
​
function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
...
listingsCounter++;
activeListingsCounter++;
​
- s_listings[_tokenId] =
+ s_listings[listingsCounter] =
Listing({seller: msg.sender, price: _price, nft: address(this), tokenId: _tokenId, isActive: true});
emit NFT_Dealers_Listed(msg.sender, listingsCounter);
}
​
- require(s_listings[_tokenId].isActive == false, "NFT is already listed");
+ // Track active listing by token separately
+ require(tokenToActiveListingId[_tokenId] == 0, "NFT is already listed");
+ tokenToActiveListingId[_tokenId] = listingsCounter;
​
function buy(uint256 _listingId) external payable {
...
- s_listings[_listingId].isActive = false;
+ s_listings[_listingId].isActive = false;
+ tokenToActiveListingId[s_listings[_listingId].tokenId] = 0;
}
​
function cancelListing(uint256 _listingId) external {
...
+ tokenToActiveListingId[s_listings[_listingId].tokenId] = 0;
}

Support

FAQs

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

Give us feedback!