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

Mapping Key Confusion

Author Revealed upon completion

Root + Impact

Description

  • When a user lists an NFT, the contract emits an event with listingsCounter as the listing ID, and users see this ID in the frontend to identify which listing they want to interact with.

  • The mapping stores listings using tokenId as the key, but all the marketplace functions expect listingId as the parameter, so when users try to buy/cancel/collect using the ID from the event, they get the wrong listing or an empty one.

solidity

function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
listingsCounter++;
activeListingsCounter++;
​
@> s_listings[_tokenId] = Listing({seller: msg.sender, price: _price, nft: address(this), tokenId: _tokenId, isActive: true});
@> emit NFT_Dealers_Listed(msg.sender, listingsCounter); // emits counter, stores at tokenId
}
​
function buy(uint256 _listingId) external payable {
@> Listing memory listing = s_listings[_listingId]; // tries to read with listingId
if (!listing.isActive) revert ListingNotActive(_listingId);
// ...
}

Risk

Likelihood:

  • Users get the listing ID from the event and use it to call buy(), cancelListing(), updatePrice(), or collectUsdcFromSelling()

  • The function looks up s_listings[listingId] but the data is stored at s_listings[tokenId], so it returns empty data or wrong listing

Impact:

  • Nobody can buy NFTs using the listing ID shown in events or frontend

  • The entire marketplace is broken because the key mismatch makes listings inaccessible

  • Same issue affects cancelListing, updatePrice, and collectUsdcFromSelling - all unusable

Proof of Concept

solidity

// Alice mints tokenId #10 (she already minted 9 others)
// Alice lists her NFT
list(10, 100e6);
// Storage: s_listings[10] = {seller: alice, price: 100, ...}
// Event: NFT_Dealers_Listed(alice, 1) // listingsCounter = 1
​
// Bob sees the event and tries to buy listing #1
buy(1);
// Code runs: s_listings[1]
// Returns: empty listing or wrong NFT (if tokenId #1 exists)
// Result: reverts with ListingNotActive or buys wrong NFT
​
// Bob has no way to know he needs to use tokenId #10 instead of listingId #1

Recommended Mitigation

diff

- mapping(uint256 => Listing) public s_listings;
+ mapping(uint256 listingId => Listing) public s_listings;
​
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] = Listing({seller: msg.sender, price: _price, nft: address(this), tokenId: _tokenId, isActive: true});
+ 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!