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

Listing ID vs. Token ID Mismatch

Author Revealed upon completion

Root + Impact

Description

  • The ID emitted in the NFT_Dealers_Listed event should be the same ID used to interact with the listing (e.g., in buy() or cancelListing()).

  • The event emits an incrementing listingsCounter, but the protocol functions expect the tokenId. Since users can list tokens in any order (e.g., listing Token #5 first), these two IDs will drift apart immediately.

function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
// ... logic ...
listingsCounter++; // @> Global counter incremented
activeListingsCounter++;
​
s_listings[_tokenId] = Listing({...}); // @> Mapping indexed by Token ID
emit NFT_Dealers_Listed(msg.sender, listingsCounter); // @> Event uses Counter
}

Risk

Likelihood:

  • Any integration (UI, Bot, Indexer) that listens for NFT_Dealers_Listed will receive an ID that is invalid for the buy() function if the token being listed is not the Nth token minted.

Impact:

  • User experience breakdown: Users see "Listing #1" in the UI, but calling buy(1) either fails or buys the wrong NFT.

  • Protocol functions like buy, cancelListing, and updatePrice become essentially unusable via standard event-driven interfaces.

Proof of Concept

import {Test, Vm, console} from "forge-std/Test.sol";
function testListingIdMismatch_Bug() public revealed {
usdc.mint(userWithCash, 20e6);
// 1. Mint two NFTs
vm.prank(owner);
nftDealers.whitelistWallet(userWithCash);
vm.startBroadcast(userWithCash);
usdc.approve(address(nftDealers), 40e6);
nftDealers.mintNft(); // Token ID 1
nftDealers.mintNft(); // Token ID 2
​
// 2. List Token ID 2
// listingsCounter will be 1, but it's stored at mapping index [2]
vm.recordLogs();
nftDealers.list(2, 1000e6);
​
Vm.Log[] memory entries = vm.getRecordedLogs();
// event NFT_Dealers_Listed(address indexed listedBy, uint256 listingId);
// listingId is the first (and only) non-indexed parameter in the data
uint256 listingIdFromEvent = abi.decode(entries[0].data, (uint256));
​
assertEq(listingIdFromEvent, 1, "Event should say Listing ID is 1");
vm.stopBroadcast();
​
// 3. Buyer tries to buy the listing they saw in the event
vm.startBroadcast(userWithEvenMoreCash);
usdc.approve(address(nftDealers), 1000e6);
​
// This fails because buy(1) looks for token 1, but we listed token 2.
vm.expectRevert(abi.encodeWithSelector(NFTDealers.ListingNotActive.selector, listingIdFromEvent));
nftDealers.buy(listingIdFromEvent);
vm.stopBroadcast();
}

Recommended Mitigation

Standardize the indexing. Either use listingsCounter as the key for the s_listings mapping (allowing a token to be listed multiple times) or emit the _tokenId in the event instead of the counter.

Support

FAQs

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

Give us feedback!