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

Listings are stored by `tokenId` but the event emits `listingsCounter` — the marketplace shows wrong listing IDs

Author Revealed upon completion

Root + Impact

Description

When someone lists an NFT, the listing data goes into s_listings[_tokenId]. But the NFT_Dealers_Listed event reports listingsCounter as the listing ID — a completely different number. Every function that reads or modifies a listing (buy, cancelListing, updatePrice, collectUsdcFromSelling) looks up s_listings[_listingId], so the ID in the event doesn't match where the data actually lives.

Any frontend or indexer that uses the event to build the listing ID will give users the wrong one. They'll call buy() with the wrong ID, either getting a revert or — worse — accidentally buying a different NFT.

function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
...
s_listings[_tokenId] = Listing({...}); // @> stored at tokenId
emit NFT_Dealers_Listed(msg.sender, listingsCounter); // @> emits counter — mismatch
}

Risk

Likelihood: Happens every time tokenId ≠ listingsCounter. If the 5th minted NFT is listed first, tokenId=5 but listingsCounter=1. The event says "1" but the data is at slot 5.

Impact: The marketplace is broken for real-world use. Users will either get reverts (wasting gas) or buy the wrong NFT at the wrong price — a direct loss of funds with no way to recover.

Proof of Concept

  1. Alice mints NFTs #1, #2, #3.

  2. She lists #3 first. listingsCounter = 1, data stored at s_listings[3]. Event says listing ID = 1.

  3. Bob sees "listing 1" on the frontend, calls buy(1) — reverts because s_listings[1] is empty.

  4. Later Alice lists #1. listingsCounter = 2, data at s_listings[1]. Event says ID = 2.

  5. If Bob retries buy(1) thinking it's the original listing, he actually buys NFT #1 at a different price — not what he intended.

function testListingIdMismatch() public {
vm.prank(owner);
nftDealers.revealCollection();
vm.prank(owner);
nftDealers.whitelistWallet(alice);
​
usdc.mint(alice, 60e6);
vm.startPrank(alice);
usdc.approve(address(nftDealers), 60e6);
nftDealers.mintNft(); // tokenId 1
nftDealers.mintNft(); // tokenId 2
nftDealers.mintNft(); // tokenId 3
vm.stopPrank();
​
// List token 3 first — stored at s_listings[3], event says ID = 1
vm.prank(alice);
nftDealers.list(3, 500e6);
​
// buy(1) fails — nothing at s_listings[1]
usdc.mint(carol, 500e6);
vm.startPrank(carol);
usdc.approve(address(nftDealers), 500e6);
vm.expectRevert(abi.encodeWithSelector(NFTDealers.ListingNotActive.selector, 1));
nftDealers.buy(1);
vm.stopPrank();
​
// The real ID is 3 (the tokenId)
vm.startPrank(carol);
nftDealers.buy(3);
vm.stopPrank();
assertEq(nftDealers.ownerOf(3), carol);
}

Recommended Mitigation

Emit _tokenId in the event instead of listingsCounter, so the event matches the actual storage key.

function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
...
- emit NFT_Dealers_Listed(msg.sender, listingsCounter);
+ emit NFT_Dealers_Listed(msg.sender, _tokenId);
}

Support

FAQs

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

Give us feedback!