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

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

Author Revealed upon completion

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

Support

FAQs

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

Give us feedback!