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

s_listings is indexed by tokenId but events emit listingId, causing off-chain integrators to pass wrong IDs

Author Revealed upon completion

Root + Impact

Description

  • list() stores listings in s_listings[_tokenId] — keyed by tokenId — and all functions that interact with a listing (buy(), cancelListing(), collectUsdcFromSelling(), updatePrice()) expect a tokenId as their _listingId parameter.

  • The NFT_Dealers_Listed event emits listingsCounter as listingId, suggesting to off-chain integrators that the returned value is the key needed to interact with the listing. It is not — passing listingsCounter to any of the above functions will cause a revert or interact with the wrong listing.

function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
@> s_listings[_tokenId] = Listing({...}); // stored at key = tokenId
​
listingsCounter++;
@> emit NFT_Dealers_Listed(msg.sender, listingsCounter); // emits listingId = listingsCounter
// off-chain integrator reads listingId=1, calls buy(1) — but listing is at s_listings[tokenId]
}

Risk

Likelihood:

  • Every single list() call emits the wrong ID — this is not conditional, it happens 100% of the time without exception.

  • Any off-chain integrator, frontend, or user that reads the NFT_Dealers_Listed event and uses the emitted listingId to call buy(), cancelListing(), or updatePrice() will pass the wrong ID.

Impact:

  • Calls to buy(), cancelListing(), collectUsdcFromSelling(), and updatePrice() revert with ListingNotActive when using the emitted listingId.

  • Users and integrators have no way to discover the correct key (tokenId) from the event alone, making the protocol difficult to integrate correctly.

Proof of Concept

  1. Alice lists tokenId 3. s_listings[3] is created. Event emits listingId = 1.

  2. Bob reads the event, sees listingId = 1, and calls buy(1).

  3. s_listings[1] does not exist — transaction reverts with ListingNotActive(1).

  4. Bob must know to call buy(3) (tokenId) instead, which is not communicated anywhere.

function test_listingIdMismatch() public {
vm.prank(bob); nftDealers.mintNft(); // tokenId 1
vm.prank(bob); nftDealers.mintNft(); // tokenId 2
vm.prank(alice); nftDealers.mintNft(); // tokenId 3
​
// Alice lists tokenId 3 — stored at s_listings[3], event emits listingId=1
vm.prank(alice); nftDealers.list(3, 100e6);
​
// s_listings[3] is active, s_listings[1] is empty
(,,,, bool activeAtTokenId) = nftDealers.s_listings(3);
(,,,, bool activeAtListingId) = nftDealers.s_listings(1);
assertEq(activeAtTokenId, true); // listing stored at tokenId key
assertEq(activeAtListingId, false); // listingId key is empty
​
// Bob reads event listingId=1, calls buy(1) — reverts
vm.prank(bob);
vm.expectRevert(abi.encodeWithSignature("ListingNotActive(uint256)", uint256(1)));
nftDealers.buy(1); // wrong — listing is at s_listings[3]
​
// Only works if bob knows the tokenId out-of-band
vm.prank(bob);
nftDealers.buy(3); // correct — but not discoverable from the event alone
}

Recommended Mitigation

Either index s_listings by a dedicated counter and use that as the key across all functions, or fix the event to emit tokenId instead of listingsCounter:

- 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!