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

`list()` enforces `onlyWhitelisted` despite the specification stating non-whitelisted users can list NFTs, creating an asymmetric marketplace

Author Revealed upon completion

Root + Impact

Description

The list() function (L127-139) enforces the onlyWhitelisted modifier (L77-80), but the protocol specification explicitly states that non-whitelisted users can "buy, update price, cancel listing, list NFT, collect USDC after selling". This is the only marketplace function that contradicts the specification -- all four other functions that the spec says non-whitelisted users can access are correctly unrestricted:

Function Line Modifier Whitelist Required? Spec Says? Match?
buy() L141 none No Can use ✅
updatePrice() L185 onlySeller No Can use ✅
cancelListing() L157 none No Can use ✅
list() L127 onlyWhitelisted Yes Can use ❌
collectUsdcFromSelling() L171 onlySeller No Can use ✅

The root cause is the onlyWhitelisted modifier on list() at L127:

function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted { // @audit L127: onlyWhitelisted contradicts spec
require(_price >= MIN_PRICE, "Price must be at least 1 USDC"); // @audit L128
require(ownerOf(_tokenId) == msg.sender, "Not owner of NFT"); // @audit L129: ownership check is sufficient access control
require(s_listings[_tokenId].isActive == false, "NFT is already listed"); // @audit L130
require(_price > 0, "Price must be greater than 0"); // @audit L131
...
}

This creates an asymmetric marketplace where anyone can buy, but only whitelisted users can sell. Two realistic scenarios produce trapped NFT owners:

  1. Non-whitelisted buyer: buy() (L141) has no whitelist restriction, so any user can purchase an NFT. But the buyer cannot re-list it because list() requires whitelist status. The protocol's own marketplace creates users who can buy but not sell.

  2. De-whitelisted user: The owner can call removeWhitelistedWallet() (L106-108) at any time per the specification. A user who minted NFTs while whitelisted and is later removed from the whitelist permanently loses the ability to list their NFTs on the marketplace.

Risk

Likelihood:

  • Occurs for every non-whitelisted user who acquires an NFT -- whether via buy() (L141, no whitelist restriction), standard ERC721 transferFrom, or after being removed from the whitelist via removeWhitelistedWallet() (L106-108)

  • The owner can remove wallets from the whitelist at any time per the specification, affecting previously whitelisted users

  • buy() is completely unrestricted, so non-whitelisted buyers routinely enter the marketplace and acquire NFTs they cannot resell

Impact:

  • Non-whitelisted NFT owners cannot sell on the protocol's marketplace, contradicting the documented behavior

  • Creates a buy-only trap: users purchase NFTs via buy() expecting to participate fully in the marketplace, but cannot resell

  • De-whitelisted users lose marketplace access for NFTs they legitimately minted and own

  • NFTs are not completely locked (ERC721 transferFrom still works), but the protocol's core selling functionality is inaccessible

Proof of Concept

Two scenarios demonstrate the issue. Run with forge test --match-contract M02PoCTest -vv:

Scenario 1: Non-whitelisted buyer purchases an NFT via buy() then cannot re-list it.
Scenario 2: User mints while whitelisted, then owner removes them from whitelist -- they can no longer list.

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.34;
​
import {Test, console} from "forge-std/Test.sol";
import {NFTDealers} from "../src/NFTDealers.sol";
import {MockUSDC} from "../src/MockUSDC.sol";
​
contract M02PoCTest is Test {
NFTDealers public nftDealers;
MockUSDC public usdc;
​
address public owner = makeAddr("owner");
address public seller = makeAddr("seller");
address public buyer = makeAddr("buyer"); // deliberately NOT whitelisted
​
uint256 constant LOCK_AMOUNT = 20e6; // 20 USDC
​
function setUp() public {
usdc = new MockUSDC();
nftDealers = new NFTDealers(owner, address(usdc), "NFTDealers", "NFTD", "ipfs://test/", LOCK_AMOUNT);
​
vm.startPrank(owner);
nftDealers.revealCollection();
nftDealers.whitelistWallet(seller);
// buyer is NOT whitelisted -- per README, they should still be able to list
vm.stopPrank();
​
usdc.mint(seller, 1000e6);
usdc.mint(buyer, 1000e6);
​
vm.prank(seller);
usdc.approve(address(nftDealers), type(uint256).max);
vm.prank(buyer);
usdc.approve(address(nftDealers), type(uint256).max);
}
​
function test_nonWhitelistedBuyerCannotRelist() public {
// --- Seller mints and lists ---
vm.prank(seller);
nftDealers.mintNft(); // tokenId 1
vm.prank(seller);
nftDealers.list(1, uint32(100e6)); // list for 100 USDC
​
// --- Non-whitelisted buyer purchases via buy() -- no whitelist required ---
assertFalse(nftDealers.isWhitelisted(buyer));
vm.prank(buyer);
nftDealers.buy(1); // succeeds: buy() has no onlyWhitelisted modifier
​
// Buyer now owns the NFT
assertEq(nftDealers.ownerOf(1), buyer);
​
// --- Buyer tries to re-list on the marketplace -- REVERTS ---
vm.prank(buyer);
vm.expectRevert("Only whitelisted users can call this function");
nftDealers.list(1, uint32(150e6));
​
console.log("Non-whitelisted buyer owns NFT but cannot list it on the marketplace");
}
​
function test_dewhitelistedUserCannotList() public {
// --- User mints while whitelisted ---
vm.prank(seller);
nftDealers.mintNft(); // tokenId 1
​
// --- Owner removes seller from whitelist ---
vm.prank(owner);
nftDealers.removeWhitelistedWallet(seller);
assertFalse(nftDealers.isWhitelisted(seller));
​
// --- Formerly-whitelisted user tries to list -- REVERTS ---
vm.prank(seller);
vm.expectRevert("Only whitelisted users can call this function");
nftDealers.list(1, uint32(100e6));
​
console.log("De-whitelisted user owns NFT but cannot list it on the marketplace");
}
}

Recommended Mitigation

Remove the onlyWhitelisted modifier from list() to align with the specification. The ownerOf(_tokenId) == msg.sender check at L129 already provides sufficient access control -- only the actual NFT owner can list their token.

-function list(uint256 _tokenId, uint32 _price) external onlyWhitelisted {
+function list(uint256 _tokenId, uint32 _price) external {
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});
emit NFT_Dealers_Listed(msg.sender, listingsCounter);
}

This fix:

  1. Aligns list() with the specification and with all other marketplace functions (buy, cancelListing, updatePrice, collectUsdcFromSelling) which are already unrestricted

  2. Preserves the ownership check at L129 (ownerOf(_tokenId) == msg.sender) as the access control mechanism

  3. Allows non-whitelisted buyers who acquired NFTs via buy() to participate fully in the marketplace

  4. Allows users removed from the whitelist to still manage and sell their existing NFTs

Support

FAQs

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

Give us feedback!