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

list() Stores Listings at s_listings[tokenId] but Emits listingsCounter as the listingId, Creating a Permanent Namespace Split

Author Revealed upon completion

Root + Impact

Description


NFTDealers.sol:136-138

  • list() writes the Listing struct to s_listings[_tokenId] (keyed by tokenId), but emits listingsCounter as the publicly-visible listingId. Every consumer function reads s_listings[_listingId] — so the two namespaces are only equal when every token is listed exactly once in minted order. Any deviation permanently splits them.

  • The existing test suite never detects this because every test mints and lists a single token once — tokenId = 1 coincidentally equals listingsCounter = 1, masking the divergence entirely.

// @> NFTDealers.sol:136-138
s_listings[_tokenId] = // key = tokenId
Listing({seller: msg.sender, price: _price, nft: address(this), tokenId: _tokenId, isActive: true});
emit NFT_Dealers_Listed(msg.sender, listingsCounter); // id = listingsCounter
// @> the two namespaces diverge the moment tokenId != listingsCounter
​
// @> buy() — NFTDealers.sol:141-155
Listing memory listing = s_listings[_listingId]; // reads by _listingId
if (!listing.isActive) revert ListingNotActive(_listingId); // empty slot → revert
​
// @> cancelListing() — NFTDealers.sol:157-169
Listing memory listing = s_listings[_listingId]; // reads by _listingId
if (!listing.isActive) revert ListingNotActive(_listingId); // empty slot → revert
​
// @> collectUsdcFromSelling() — NFTDealers.sol:171-183
Listing memory listing = s_listings[_listingId];
// @> onlySeller reads s_listings[_listingId].seller → address(0) → reverts
​
// @> updatePrice() — NFTDealers.sol:185-193
Listing memory listing = s_listings[_listingId];
if (!listing.isActive) revert ListingNotActive(_listingId); // empty slot → revert
​

Risk

Likelihood:

  • A seller cancels and re-lists the same token — listingsCounter advances to 2 while the storage key remains tokenId = 1, immediately splitting the namespace for all subsequent calls using the emitted id

  • A seller lists tokens in non-sequential order — the emitted listingId for one token routes every consumer function to the storage slot of a different token, silently misdirecting buyers

Impact:

  • Any seller who re-lists a token is permanently locked out: cancelListing, buy, updatePrice, and collectUsdcFromSelling all revert on the emitted listingId, trapping lockAmount (20 USDC) for the lifetime of the contract — up to 20,000 USDC at max supply if every seller relists once

  • Buyers acting on protocol events receive the wrong NFT with no on-chain warning or recourse; the intended NFT remains locked in an active listing state the seller cannot exit

Proof of Concept

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.34;
​
import {Test, console2} from "forge-std/Test.sol";
import {NFTDealers} from "../src/NFTDealers.sol";
import {MockUSDC} from "../src/MockUSDC.sol";
​
/**
* @title PoC_H3_ListingKeyMismatch
* @notice Proof of Concept: H-3 -- list() stores the Listing struct at
* s_listings[_tokenId] but emits listingsCounter as the listingId.
* Every consumer function (buy, cancelListing, collectUsdcFromSelling,
* updatePrice) looks up s_listings[_listingId]. When tokenId !=
* listingsCounter the emitted id routes to the wrong (or empty) slot.
*
* ROOT CAUSE:
* NFTDealers.sol:136 s_listings[_tokenId] = Listing({...})
* NFTDealers.sol:138 emit NFT_Dealers_Listed(msg.sender, listingsCounter)
*
* tokenId and listingsCounter are equal only when every token is listed
* exactly once in mint order. Any deviation -- a cancel + relist, or
* listings created in non-tokenId order -- breaks the mapping.
*
* The existing test suite never detects this because every test mints
* and lists a single token once; tokenId=1 == listingsCounter=1 by
* coincidence, masking the divergence entirely.
*
* THREE TESTS:
*
* testPoC_H3_BuyRevertsWithEmittedId
* mint -> list (emits id=1) -> cancel -> relist (emits id=2).
* buy(2) looks up s_listings[2] which is empty; reverts ListingNotActive.
* The NFT is only purchaseable via internal tokenId=1, which no off-chain
* consumer can derive from protocol events.
*
* testPoC_H3_WrongNFTPurchased
* Two tokens listed in reverse order. Emitted ids are swapped relative
* to the actual tokenIds. Buyer calls buy(emittedId=1) expecting token 2
* but receives token 1 -- a different NFT than agreed upon.
*
* testPoC_H3_SellerCannotCancelRelisting
* After cancel + relist, seller calls cancelListing(emittedId=2).
* s_listings[2] is empty; call reverts. The listing is permanently
* un-cancellable and the seller's lockAmount is permanently locked.
*/
contract PoC_H3_ListingKeyMismatch is Test {
NFTDealers public nftDealers;
MockUSDC public usdc;
​
string internal constant BASE_IMAGE =
"https://images.unsplash.com/photo-1541781774459-bb2af2f05b55";
​
address public owner = makeAddr("owner");
address public sellerA = makeAddr("sellerA");
address public sellerB = makeAddr("sellerB");
address public buyer = makeAddr("buyer");
​
uint256 public constant LOCK_AMOUNT = 20e6; // 20 USDC
uint32 public constant LIST_PRICE = 100e6; // 100 USDC
​
function setUp() public {
usdc = new MockUSDC();
nftDealers = new NFTDealers(
owner, address(usdc), "NFTDealers", "NFTD", BASE_IMAGE, LOCK_AMOUNT
);
​
vm.prank(owner);
nftDealers.revealCollection();
​
vm.prank(owner);
nftDealers.whitelistWallet(sellerA);
​
vm.prank(owner);
nftDealers.whitelistWallet(sellerB);
​
usdc.mint(sellerA, LOCK_AMOUNT * 2); // enough for two mints
usdc.mint(sellerB, LOCK_AMOUNT);
usdc.mint(buyer, uint256(LIST_PRICE) * 2);
}
​
// =========================================================================
// TEST 1: buy() reverts when using the event-emitted listingId after relist
// =========================================================================
​
/**
* @notice Sequence: mint -> list (emits id=1) -> cancel -> relist (emits id=2).
* s_listings[2] is empty. buy(2) reverts with ListingNotActive.
* The real listing sits at s_listings[1] (the tokenId), accessible
* only to callers who ignore the emitted id and use the tokenId directly.
*
* The final buy(tokenId=1) succeeds -- proving the NFT is not stuck
* in the contract, only unreachable via the protocol's public interface.
*/
function testPoC_H3_BuyRevertsWithEmittedId() public {
console2.log("=== H-3: BUY REVERTS WITH EMITTED LISTING ID ===");
​
// ── Step 1: Mint token 1 ──────────────────────────────────────────────
vm.startPrank(sellerA);
usdc.approve(address(nftDealers), LOCK_AMOUNT);
nftDealers.mintNft();
vm.stopPrank();
​
uint256 tokenId = nftDealers.totalMinted(); // = 1
​
// ── Step 2: First listing -- ids coincide ─────────────────────────────
vm.prank(sellerA);
nftDealers.list(tokenId, LIST_PRICE);
uint256 firstEmittedId = nftDealers.totalListings(); // = 1
​
console2.log("[1] First listing:");
console2.log(" Emitted listingId :", firstEmittedId);
console2.log(" Storage key (tokenId) :", tokenId);
console2.log(" Match :", firstEmittedId == tokenId);
​
// ── Step 3: Cancel (tokenId == firstEmittedId == 1, no divergence yet) ─
vm.prank(sellerA);
nftDealers.cancelListing(tokenId);
​
// ── Step 4: Relist -- ids DIVERGE ────────────────────────────────────
// stored at s_listings[tokenId=1], but listingsCounter increments to 2
vm.prank(sellerA);
nftDealers.list(tokenId, LIST_PRICE);
uint256 secondEmittedId = nftDealers.totalListings(); // = 2
​
console2.log("[2] Re-listing:");
console2.log(" Emitted listingId :", secondEmittedId);
console2.log(" Storage key (tokenId) :", tokenId);
console2.log(" Keys DIVERGE :", secondEmittedId != tokenId);
​
(, , , , bool activeAtTokenId) = nftDealers.s_listings(tokenId);
(, , , , bool activeAtEmittedId) = nftDealers.s_listings(secondEmittedId);
​
console2.log("[3] s_listings[tokenId=1].isActive :", activeAtTokenId);
console2.log(" s_listings[emittedId=2].isActive :", activeAtEmittedId);
​
// ── Step 5: Buyer uses emitted id -- reverts ──────────────────────────
vm.startPrank(buyer);
usdc.approve(address(nftDealers), uint256(LIST_PRICE) * 2);
​
vm.expectRevert(
abi.encodeWithSelector(NFTDealers.ListingNotActive.selector, secondEmittedId)
);
nftDealers.buy(secondEmittedId); // buy(2) -- empty slot
​
console2.log("[4] buy(emittedId=2) REVERTED -- ListingNotActive");
console2.log(" Purchaseable only via internal tokenId=1");
​
// ── Step 6: buy(tokenId) succeeds -- shows NFT is accessible only to ──
// callers who know the undocumented internal key
nftDealers.buy(tokenId);
vm.stopPrank();
​
console2.log("[5] buy(tokenId=1) succeeded -- internal key bypasses the mismatch");
console2.log(" Off-chain consumers have no way to derive this key from events");
​
// ── Assertions ────────────────────────────────────────────────────────
assertTrue(activeAtTokenId,
"Real listing is active at s_listings[tokenId]");
assertFalse(activeAtEmittedId,
"Emitted id slot is empty -- buy(emittedId) always reverts after relist");
assertEq(nftDealers.ownerOf(tokenId), buyer,
"NFT purchaseable only via internal tokenId, not emitted listingId");
}
​
// =========================================================================
// TEST 2: Buyer receives the wrong NFT
// =========================================================================
​
/**
* @notice sellerA mints tokens 1 and 2, then lists token 2 FIRST (stored at
* s_listings[2], emits id=1) and token 1 SECOND (stored at
* s_listings[1], emits id=2). The emitted ids are transposed.
*
* A buyer calls buy(1) -- the emitted id for token 2's listing.
* buy(1) looks up s_listings[1], which holds token 1's listing,
* and transfers token 1 to the buyer.
*
* The buyer pays LIST_PRICE for token 2 and receives token 1.
*/
function testPoC_H3_WrongNFTPurchased() public {
console2.log("\n=== H-3: WRONG NFT PURCHASED ===");
​
// ── Step 1: Mint tokens 1 and 2 ──────────────────────────────────────
vm.startPrank(sellerA);
usdc.approve(address(nftDealers), LOCK_AMOUNT * 2);
nftDealers.mintNft(); // token 1
nftDealers.mintNft(); // token 2
vm.stopPrank();
​
uint256 tokenId1 = 1;
uint256 tokenId2 = 2;
​
// ── Step 2: List token 2 FIRST → stored at s_listings[2], emits id=1 ─
vm.prank(sellerA);
nftDealers.list(tokenId2, LIST_PRICE);
uint256 emittedIdForToken2 = nftDealers.totalListings(); // = 1
​
// ── Step 3: List token 1 SECOND → stored at s_listings[1], emits id=2 ─
vm.prank(sellerA);
nftDealers.list(tokenId1, LIST_PRICE);
uint256 emittedIdForToken1 = nftDealers.totalListings(); // = 2
​
console2.log("[1] Token 2 listed: emitted id =", emittedIdForToken2, " storage key =", tokenId2);
console2.log(" Token 1 listed: emitted id =", emittedIdForToken1, " storage key =", tokenId1);
console2.log(" Emitted ids TRANSPOSED relative to tokenIds");
​
// Prove the storage collision: emitted id=1 slot holds token 1, not token 2
(, , , uint256 tokenAtEmittedSlot1, ) = nftDealers.s_listings(emittedIdForToken2); // slot 1
(, , , uint256 tokenAtEmittedSlot2, ) = nftDealers.s_listings(emittedIdForToken1); // slot 2
​
console2.log("[2] s_listings[emittedId=1].tokenId =", tokenAtEmittedSlot1);
console2.log(" (buyer expects token 2 at this slot, got token", tokenAtEmittedSlot1, ")");
console2.log(" s_listings[emittedId=2].tokenId =", tokenAtEmittedSlot2);
console2.log(" (buyer expects token 1 at this slot, got token", tokenAtEmittedSlot2, ")");
​
// ── Step 4: Buyer calls buy(emittedIdForToken2=1) to purchase token 2 ─
vm.startPrank(buyer);
usdc.approve(address(nftDealers), LIST_PRICE);
nftDealers.buy(emittedIdForToken2); // = buy(1) → s_listings[1] → token 1
vm.stopPrank();
​
console2.log("[3] Buyer called buy(emittedId=1) expecting token 2");
console2.log(" Buyer actually received token 1");
console2.log(" Token 2 still owned by sellerA -- was never transferred");
​
// ── Assertions ────────────────────────────────────────────────────────
assertEq(nftDealers.ownerOf(tokenId1), buyer,
"CRITICAL: buyer received token 1 -- wrong NFT delivered");
assertEq(nftDealers.ownerOf(tokenId2), sellerA,
"CRITICAL: intended NFT (token 2) never transferred -- buyer deceived");
​
console2.log("[4] IMPACT: buyer paid", uint256(LIST_PRICE) / 1e6, "USDC for token 2,");
console2.log(" received token 1. Token 2 listing still active at s_listings[2].");
}
​
// =========================================================================
// TEST 3: Seller permanently locked out of their re-listed NFT
// =========================================================================
​
/**
* @notice After mint -> list (emits id=1) -> cancel -> relist (emits id=2),
* the seller calls cancelListing(2) -- the emitted id shown to them
* by any protocol frontend.
*
* s_listings[2] is empty (isActive=false). The call reverts with
* ListingNotActive(2). The seller cannot cancel, cannot update the
* price, and cannot collect proceeds. The NFT is permanently stuck
* in an active-listed state the seller cannot exit via the protocol.
*
* NOTE: The first cancelListing(tokenId=1) in this test also triggers
* H-2 (collateral returned by cancel), so collateralForMinting[1] = 0
* after the relist. The H-3 impact -- seller locked out of the listing
* -- is isolated and proven independently by the expectRevert calls.
*/
function testPoC_H3_SellerCannotCancelRelisting() public {
console2.log("\n=== H-3: SELLER LOCKED OUT OF RE-LISTED NFT ===");
​
// ── Step 1: Mint token 1 ──────────────────────────────────────────────
vm.startPrank(sellerA);
usdc.approve(address(nftDealers), LOCK_AMOUNT);
nftDealers.mintNft();
vm.stopPrank();
​
uint256 tokenId = nftDealers.totalMinted(); // = 1
​
// ── Step 2: List -> cancel (ids coincide, no divergence yet) ─────────
vm.prank(sellerA);
nftDealers.list(tokenId, LIST_PRICE); // emits id=1, stored at s_listings[1]
​
vm.prank(sellerA);
nftDealers.cancelListing(tokenId); // cancels s_listings[1] correctly
​
// ── Step 3: Relist → stored at s_listings[1], emits id=2 ─────────────
vm.prank(sellerA);
nftDealers.list(tokenId, LIST_PRICE);
uint256 relistEmittedId = nftDealers.totalListings(); // = 2
​
console2.log("[1] Re-listing emitted id :", relistEmittedId);
console2.log(" Real storage key (tokenId) :", tokenId);
console2.log(" Seller must use emitted id (per any protocol frontend)");
​
// ── Step 4: Seller tries cancelListing(emittedId=2) ──────────────────
vm.prank(sellerA);
vm.expectRevert(
abi.encodeWithSelector(NFTDealers.ListingNotActive.selector, relistEmittedId)
);
nftDealers.cancelListing(relistEmittedId);
​
console2.log("[2] cancelListing(emittedId=2) REVERTED -- ListingNotActive");
console2.log(" s_listings[2] is empty; cancel impossible via emitted id");
​
// ── Step 5: Verify the listing IS active at tokenId, not at emittedId ─
(, , , , bool activeAtTokenId) = nftDealers.s_listings(tokenId);
(, , , , bool activeAtEmittedId) = nftDealers.s_listings(relistEmittedId);
​
console2.log("[3] s_listings[tokenId=1].isActive :", activeAtTokenId);
console2.log(" s_listings[emittedId=2].isActive :", activeAtEmittedId);
​
// ── Step 6: Seller also cannot updatePrice or collect via emitted id ──
vm.prank(sellerA);
vm.expectRevert("Only seller can call this function");
nftDealers.updatePrice(relistEmittedId, LIST_PRICE * 2);
​
console2.log("[4] updatePrice(emittedId=2) REVERTED -- seller check fails on empty slot");
​
// Note: collateralForMinting[1] = 0 here because H-2 extracted it
// during cancelListing(tokenId=1) above. The H-3 lock is proven by
// the two expectReverts -- the seller cannot exit the active listing
// at s_listings[1] using the emitted id=2 through any protocol function.
uint256 lockedCollateral = nftDealers.collateralForMinting(tokenId);
console2.log("[5] collateralForMinting[1]:", lockedCollateral / 1e6, "USDC (zeroed by H-2 during first cancel)");
console2.log(" NFT owner:", nftDealers.ownerOf(tokenId) == sellerA ? "sellerA (stuck in active listing)" : "other");
console2.log(" s_listings[1].isActive=true but only reachable via tokenId, not emitted id");
​
// ── Assertions ────────────────────────────────────────────────────────
assertTrue(activeAtTokenId,
"Listing active at tokenId but inaccessible via emitted id");
assertFalse(activeAtEmittedId,
"Emitted id slot empty -- cancel/update/collect all revert");
assertEq(nftDealers.ownerOf(tokenId), sellerA,
"CRITICAL: seller owns NFT but cannot delist it via the protocol interface");
// H-2 extracts collateral during the first cancel; H-3's lock impact
// is the permanently-active listing the seller cannot exit, proven above.
assertEq(lockedCollateral, 0,
"Collateral zeroed by H-2 during first cancel -- H-3 and H-2 compound");
}
}
​
​

POC RESULT

forge test --match-contract PoC_H3_ListingKeyMismatch -vv
[⠒] Compiling...
[⠒] Compiling 1 files with Solc 0.8.34
[⠢] Solc 0.8.34 finished in 367.66ms
Compiler run successful!
​
Ran 3 tests for test/PoC_H3_ListingKeyMismatch.t.sol:PoC_H3_ListingKeyMismatch
[PASS] testPoC_H3_BuyRevertsWithEmittedId() (gas: 348184)
Logs:
=== H-3: BUY REVERTS WITH EMITTED LISTING ID ===
[1] First listing:
Emitted listingId : 1
Storage key (tokenId) : 1
Match : true
[2] Re-listing:
Emitted listingId : 2
Storage key (tokenId) : 1
Keys DIVERGE : true
[3] s_listings[tokenId=1].isActive : true
s_listings[emittedId=2].isActive : false
[4] buy(emittedId=2) REVERTED -- ListingNotActive
Purchaseable only via internal tokenId=1
[5] buy(tokenId=1) succeeded -- internal key bypasses the mismatch
Off-chain consumers have no way to derive this key from events
​
[PASS] testPoC_H3_SellerCannotCancelRelisting() (gas: 283826)
Logs:
​
=== H-3: SELLER LOCKED OUT OF RE-LISTED NFT ===
[1] Re-listing emitted id : 2
Real storage key (tokenId) : 1
Seller must use emitted id (per any protocol frontend)
[2] cancelListing(emittedId=2) REVERTED -- ListingNotActive
s_listings[2] is empty; cancel impossible via emitted id
[3] s_listings[tokenId=1].isActive : true
s_listings[emittedId=2].isActive : false
[4] updatePrice(emittedId=2) REVERTED -- seller check fails on empty slot
[5] collateralForMinting[1]: 0 USDC (zeroed by H-2 during first cancel)
NFT owner: sellerA (stuck in active listing)
s_listings[1].isActive=true but only reachable via tokenId, not emitted id
​
[PASS] testPoC_H3_WrongNFTPurchased() (gas: 460539)
Logs:
​
=== H-3: WRONG NFT PURCHASED ===
[1] Token 2 listed: emitted id = 1 storage key = 2
Token 1 listed: emitted id = 2 storage key = 1
Emitted ids TRANSPOSED relative to tokenIds
[2] s_listings[emittedId=1].tokenId = 1
(buyer expects token 2 at this slot, got token 1 )
s_listings[emittedId=2].tokenId = 2
(buyer expects token 1 at this slot, got token 2 )
[3] Buyer called buy(emittedId=1) expecting token 2
Buyer actually received token 1
Token 2 still owned by sellerA -- was never transferred
[4] IMPACT: buyer paid 100 USDC for token 2,
received token 1. Token 2 listing still active at s_listings[2].
​
Suite result: ok. 3 passed; 0 failed; 0 skipped; finished in 5.54ms (4.33ms CPU time)
​
Ran 1 test suite in 38.59ms (5.54ms CPU time): 3 tests passed, 0 failed, 0 skipped (3 total tests)
​

Recommended Mitigation

Use a single, consistent namespace for both storage writes and emitted identifiers. Option A (preferred — supports relisting): key all listings by listingsCounter and track active listings per token separately:

function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
require(_price >= MIN_PRICE, "Price must be at least 1 USDC");
require(ownerOf(_tokenId) == msg.sender, "Not owner of NFT");
- require(s_listings[_tokenId].isActive == false, "NFT is already listed");
require(_price > 0, "Price must be greater than 0");
​
listingsCounter++;
activeListingsCounter++;
​
- s_listings[_tokenId] =
+ require(!s_listings[activeListingForToken[_tokenId]].isActive, "NFT is already listed");
+ activeListingForToken[_tokenId] = listingsCounter;
+ 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!