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

[C-2] Listing Key Is tokenId but Emitted Event ID Is listingsCounter - Off-Chain Integration Broken

Author Revealed upon completion

Root + Impact

Description

  • list() is expected to store a listing that callers can reference by a stable listing ID emitted in the event. All read and write functions (buy, cancelListing, updatePrice, collectUsdcFromSelling) accept a _listingId that maps directly to s_listings[_listingId].

  • The listing is stored at s_listings[_tokenId] but the event emits listingsCounter as the listing ID. These two values diverge as soon as a token with ID ≠ 1 is listed first, or when the counter and token ID differ. Any frontend or integration that calls buy(emittedListingId) will operate on the wrong listing entry.

// src/NFTDealers.sol
function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
...
listingsCounter++;
activeListingsCounter++;
​
@> s_listings[_tokenId] = Listing({...}); // keyed by tokenId
@> emit NFT_Dealers_Listed(msg.sender, listingsCounter); // emits counter, not tokenId
}

Risk

Likelihood:

  • Any token listed with a tokenId that does not equal its position in the mint order produces a mismatched event.

  • The mismatch grows over time as listings are cancelled, tokens are transferred, and re-listed.

Impact:

  • Buyers calling buy(listingId) from event data attempt to purchase an unrelated or non-existent listing.

  • Off-chain indexers, marketplaces, and UIs built on the event present incorrect data to users.

Proof of Concept

Two tokens are minted, the second one is listed first. The event emits listingId=1 (from listingsCounter) but the listing is stored at s_listings[2] (from _tokenId). A buyer acting on the emitted ID reverts.

function testListingIdMismatch() public revealed {
mintNFTForTesting(); // tokenId = 1
mintNFTForTesting(); // tokenId = 2
​
vm.prank(owner);
nftDealers.whitelistWallet(userWithEvenMoreCash);
​
// userWithEvenMoreCash lists tokenId=2 → listingsCounter becomes 1
// Event emits listingId=1, but listing stored at s_listings[2]
vm.startPrank(userWithEvenMoreCash);
nftDealers.list(2, 1000e6);
vm.stopPrank();
​
// A buyer sees event listingId=1 and calls buy(1)
// s_listings[1] is empty/inactive → reverts unexpectedly
vm.startPrank(userWithCash);
usdc.approve(address(nftDealers), 1000e6);
vm.expectRevert(); // ListingNotActive(1) — wrong listing queried
nftDealers.buy(1);
vm.stopPrank();
}

Recommended Mitigation

Use listingsCounter as both the storage key and the emitted ID, and keep a reverse mapping from listing ID to tokenId so internal functions can still look up the token.

+ mapping(uint256 => uint256) public listingIdToTokenId;
​
function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
...
listingsCounter++;
activeListingsCounter++;
​
- s_listings[_tokenId] = Listing({...});
- emit NFT_Dealers_Listed(msg.sender, listingsCounter);
+ s_listings[listingsCounter] = Listing({..., tokenId: _tokenId});
+ listingIdToTokenId[listingsCounter] = _tokenId;
+ 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!