Description
-
According to the README, the protocol defines three actor types, and non-whitelisted users/wallets are explicitly documented as being able to buy, update price, cancel listing, list NFT, and collect USDC after selling, while only minting is reserved for whitelisted users.
-
The implementation contradicts that documented behavior because list() is protected by onlyWhitelisted, which means a non-whitelisted user who acquires an NFT cannot create a secondary-market listing for it. In practice, the code allows non-whitelisted users to buy NFTs, but then prevents them from listing the NFT they now own, which directly conflicts with the stated actor model in the spec.
modifier onlyWhitelisted() {
require(whitelistedUsers[msg.sender], "Only whitelisted users can call this function");
_;
}
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");
...
}
function buy(uint256 _listingId) external payable {
Listing memory listing = s_listings[_listingId];
if (!listing.isActive) revert ListingNotActive(_listingId);
require(listing.seller != msg.sender, "Seller cannot buy their own NFT");
...
}
Risk
Likelihood: Medium
-
This occurs whenever a non-whitelisted user buys an NFT and then attempts to resell it, because buy() is open to them but list() is not.
-
This occurs during normal marketplace usage because secondary-market resale by buyers is a core flow described in the protocol design.
Impact: Medium
-
The implementation breaks a documented user role and prevents non-whitelisted owners from participating in the secondary market as the spec says they should.
-
Frontends, integrators, and users relying on the README can be misled into expecting resale functionality that the contract actually rejects on-chain, causing failed transactions and broken UX.
Proof of Concept
-
Add import {console2} from "forge-std/console2.sol"; at the top of NFTDealersTest.t.sol.
-
Copy the code below to NFTDealersTest contract.
-
Run command forge test --mt testImplementationContradictsSpecForNonWhitelistedUsers -vv --via-ir.
function testImplementationContradictsSpecForNonWhitelistedUsers() public revealed whitelisted {
uint256 tokenId = 1;
uint32 price = 1000e6;
mintAndListNFTForTesting(tokenId, price);
console2.log("Whitelisted seller:", userWithCash);
console2.log("Non-whitelisted buyer:", userWithEvenMoreCash);
console2.log("Seller is whitelisted:", nftDealers.isWhitelisted(userWithCash) ? uint256(1) : uint256(0));
console2.log(
"Buyer is whitelisted before purchase:",
nftDealers.isWhitelisted(userWithEvenMoreCash) ? uint256(1) : uint256(0)
);
console2.log("Owner before purchase:", nftDealers.ownerOf(tokenId));
assertTrue(nftDealers.isWhitelisted(userWithCash), "sanity: seller should be whitelisted");
assertFalse(nftDealers.isWhitelisted(userWithEvenMoreCash), "sanity: buyer should not be whitelisted");
assertEq(nftDealers.ownerOf(tokenId), userWithCash, "sanity: seller owns token before purchase");
vm.startPrank(userWithEvenMoreCash);
usdc.approve(address(nftDealers), price);
nftDealers.buy(tokenId);
vm.stopPrank();
console2.log("Owner after purchase:", nftDealers.ownerOf(tokenId));
console2.log(
"Buyer is whitelisted after purchase:",
nftDealers.isWhitelisted(userWithEvenMoreCash) ? uint256(1) : uint256(0)
);
assertEq(
nftDealers.ownerOf(tokenId), userWithEvenMoreCash, "sanity: non-whitelisted user can buy and own the NFT"
);
assertFalse(nftDealers.isWhitelisted(userWithEvenMoreCash), "sanity: buying does not make the user whitelisted");
vm.startPrank(userWithEvenMoreCash);
vm.expectRevert("Only whitelisted users can call this function");
nftDealers.list(tokenId, price);
vm.stopPrank();
console2.log("Non-whitelisted owner could buy token #1, but could not relist it");
console2.log("Final owner of token #1:", nftDealers.ownerOf(tokenId));
console2.log(
"Final whitelist status of owner:", nftDealers.isWhitelisted(userWithEvenMoreCash) ? uint256(1) : uint256(0)
);
assertEq(nftDealers.ownerOf(tokenId), userWithEvenMoreCash, "buyer remains the owner");
assertFalse(
nftDealers.isWhitelisted(userWithEvenMoreCash), "buyer remains non-whitelisted when attempting resale"
);
}
Output:
[⠊] Compiling...
No files changed, compilation skipped
Ran 1 test for test/NFTDealersTest.t.sol:NFTDealersTest
[PASS] testImplementationContradictsSpecForNonWhitelistedUsers() (gas: 453267)
Logs:
Whitelisted seller: 0x22CdC71E987473D657FCe79C9C0C0B1A62148056
Non-whitelisted buyer: 0x533575789af8F38A73C7747E36C17C1835FDF44a
Seller is whitelisted: 1
Buyer is whitelisted before purchase: 0
Owner before purchase: 0x22CdC71E987473D657FCe79C9C0C0B1A62148056
Owner after purchase: 0x533575789af8F38A73C7747E36C17C1835FDF44a
Buyer is whitelisted after purchase: 0
Non-whitelisted owner could buy token #1, but could not relist it
Final owner of token #1: 0x533575789af8F38A73C7747E36C17C1835FDF44a
Final whitelist status of owner: 0
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.35ms (988.00µs CPU time)
Ran 1 test suite in 12.13ms (3.35ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Recommended Mitigation
If the README is the intended behavior, the contract should remove whitelist gating from list() so that any legitimate NFT owner can create a resale listing, while minting remains restricted to whitelisted users.
-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);
}
