Bid Beasts

First Flight #49
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Impact: low
Likelihood: medium
Invalid

Unchecked NFT Existence Before the Listing

Root + Impact

Description

  • Normal behaviour: The marketplace should only allow owners of NFTs to list them.

  • Issue: If the NFT with the specified tokenId does not exist yet, ownerOf(tokenId) reverts with ERC721NonexistentToken before the ownership check, causing unexpected errors and inconsistent messages

function listNFT(uint256 tokenId, uint256 _minPrice, uint256 _buyNowPrice) external {
@> require(BBERC721.ownerOf(tokenId) == msg.sender, "Not the owner"); // Reverts if token doesn't exist
require(_minPrice >= S_MIN_NFT_PRICE, "Min price too low");
if (_buyNowPrice > 0) {
require(_minPrice <= _buyNowPrice, "Min price cannot exceed buy now price");
}

Risk

Likelihood:

  • This occurs when a user tries to list a token that has not yet been minted.

  • This occurs when frontend scripts pass invalid token IDs.

Impact:

  • Misleading error messages may confuse users or break frontends, but do not directly cause fund loss.

  • Could break frontend logic or automated scripts expecting "Not the owner".

Proof of Concept

// Using Forge test framework
function testFailListingNonExistentNFT() public {
// tokenId 0 has not been minted
vm.expectRevert("Not the owner");
market.listNFT(0, 0.01 ether, 0);
}

Explanation:
This PoC shows that attempting to list a token that has not yet been minted triggers a revert with a confusing error (ERC721NonexistentToken) rather than the expected "Not the owner" message. It shows the contract fails to handle minted tokens properly.

Recommended Mitigation

- require(BBERC721.ownerOf(tokenId) == msg.sender, "Not the owner");
+ require(BBERC721._exists(tokenId), "Token does not exist"); // check existence at the first
+ require(BBERC721.ownerOf(tokenId) == msg.sender, "Not the owner");

Explanation:

By first checking whether the token exists, the contract can give a consistent and informative error, preventing misleading reverts and improving User Experience.

Updates

Lead Judging Commences

cryptoghost Lead Judge 2 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!