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

Listing ID mismatch: storage keyed by tokenId while events emit listingsCounter breaks all marketplace operations

Author Revealed upon completion

Root + Impact

Description

list() stores the listing in s_listings[_tokenId] but emits NFT_Dealers_Listed(..., listingsCounter). All downstream functions (buy, cancelListing, updatePrice, collectUsdcFromSelling) accept a _listingId parameter and look up s_listings[_listingId]. When a user follows the event-emitted listingId (as any frontend or indexer would), the lookup hits an empty or wrong storage slot — causing reverts or purchasing the wrong NFT.

Risk

Likelihood:

  • Occurs whenever a user lists a token whose tokenId differs from the current listingsCounter value — this is the normal case when users mint multiple tokens before listing, or list tokens in non-sequential order

  • Any frontend, indexer, or direct contract interaction that reads emitted events will use the wrong ID

Impact:

  • Core marketplace functions (buy, cancelListing, updatePrice, collectUsdcFromSelling) break for any listing where tokenId ≠ listingsCounter

  • Collision scenario: if tokenId A and listingsCounter B happen to match a different listing's storage key, a buyer pays for item A but receives item B — wrong NFT purchased

  • Marketplace is unusable for normal multi-user operation

Proof of Concept

forge test --match-test test_H04_ListingIdTokenIdMismatch -vvv

Scenario: Alice mints tokens 1, 2. Carol mints token 3. Carol lists token 3 — this is listing #1 (listingsCounter = 1), but storage is at s_listings[3].

What event says Where it's stored What buy() looks up
listingId = 1 s_listings[3] s_listings[1] = empty → reverts
function test_H04_ListingIdTokenIdMismatch() public {
// Alice mints tokens 1 and 2
vm.startPrank(alice);
usdc.approve(address(nftDealers), type(uint256).max);
nftDealers.mintNft(); // tokenId 1
nftDealers.mintNft(); // tokenId 2
vm.stopPrank();
​
// Carol mints token 3
vm.startPrank(carol);
usdc.approve(address(nftDealers), type(uint256).max);
nftDealers.mintNft(); // tokenId 3
vm.stopPrank();
​
// Carol lists token 3 — this is listing #1 (listingsCounter=1)
// Event emits listingId=1, but storage is at s_listings[3] (tokenId)
vm.prank(carol);
nftDealers.list(3, uint32(100e6));
​
assertEq(nftDealers.totalListings(), 1);
​
// Bob tries to buy "listing 1" (from the event) — but s_listings[1] is empty!
vm.startPrank(bob);
usdc.approve(address(nftDealers), type(uint256).max);
vm.expectRevert(abi.encodeWithSelector(NFTDealers.ListingNotActive.selector, 1));
nftDealers.buy(1); // FAILS — listing 1 doesn't exist
vm.stopPrank();
​
// Bob needs to know to use tokenId=3, not listingId=1
vm.startPrank(bob);
nftDealers.buy(3); // This works
vm.stopPrank();
assertEq(nftDealers.ownerOf(3), bob);
}

Output: buy(1) reverts with ListingNotActive(1). buy(3) succeeds — proves storage key is tokenId, not listingsCounter.

Recommended Mitigation

Root cause — list() stores by _tokenId but emits listingsCounter:

function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
// ...
listingsCounter++;
activeListingsCounter++;
​
@> s_listings[_tokenId] = Listing({ // stored at _tokenId
seller: msg.sender,
price: _price,
nft: address(this),
tokenId: _tokenId,
isActive: true
});
@> emit NFT_Dealers_Listed(msg.sender, listingsCounter); // event emits counter
}

Fix — use one canonical key for storage, events, and function parameters:

function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
// ...
listingsCounter++;
activeListingsCounter++;
- s_listings[_tokenId] = Listing({
+ s_listings[listingsCounter] = Listing({
seller: msg.sender,
price: _price,
nft: address(this),
tokenId: _tokenId,
isActive: true
});
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!