NFT Dealers

First Flight #58
Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: medium
Valid

Non-whitelisted users cannot list the already bought NFTs while they are supposed to be able to do so.

Root + Impact

Description

  • According too the documentation, non-whitelisted users must be able to list the NFTs they have bought. However, due to the wrong application of the onlyWhitelisted modifier, they are prohibited from calling the list function.

  • By not being able too list their NFTs for sale, the bought NFTs will be locked in their wallets, unless they sell them out of the platform and by the means of direct transferring.

@> 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
});
emit NFT_Dealers_Listed(msg.sender, listingsCounter);
}

Risk

Likelihood: High

  • Because it is part of the program logic, it happens every time a non-whitelisted user tries to list their bought NFTs.


Impact: Medium

  • Even though it does not completely break the protocol, it stops some functionalities and hurts the users' feelings and trust about the protocol.


Proof of Concept

Please add the following function to the test file and run it using forge test --mt testNonWhitelistedUserCannotListBoughtNFT.

function testNonWhitelistedUserCannotListBoughtNFT() public revealed {
uint256 tokenId = 1;
uint32 nftPrice = 21e6;
mintAndListNFTForTesting(tokenId, nftPrice);
vm.prank(userWithEvenMoreCash);
vm.expectRevert("Only whitelisted users can call this function");
nftDealers.list(tokenId, nftPrice);
}

Recommended Mitigation

To solve this problem we just need to remove the onlyWhitelisted modifier from the function header, as follows.

- 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);
}
Updates

Lead Judging Commences

rubik0n Lead Judge 16 days ago
Submission Judgement Published
Validated
Assigned finding tags:

non-whitelisted-cannot-list

Support

FAQs

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

Give us feedback!