Bid Beasts

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

`src/BidBeasts_NFT_ERC721.sol: BidBeasts::burn(uint256 _tokenId)` lacks access control, allowing any caller to burn any user’s NFT

src/BidBeasts_NFT_ERC721.sol: BidBeasts::burn(uint256 _tokenId) lacks access control, allowing any caller to burn any user’s NFT

Description

  • Normally, only the NFT owner (or an authorized operator/approved address) can burn an NFT.

  • In this protocol, src/BidBeasts_NFT_ERC721.sol: BidBeasts::burn(uint256 _tokenId) lacks access control, allowing anyone to burn any NFT.

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

Risk

Likelihood:

  • This occurs whenever a malicious user calls burn(uint256 _tokenId)

Impact:

  • Any user’s NFTs can be irreversibly destroyed.

Proof of Concept

First we need to make a quick fix in test/BidBeastsMarketPlaceTest.t.sol:BidBeastsNFTMarketTest::setUp()

function setUp() public {
// Deploy contracts
- vm.prank(OWNER);
+ vm.startPrank(OWNER);
nft = new BidBeasts();
market = new BidBeastsNFTMarket(address(nft));
rejector = new RejectEther();
vm.stopPrank();
// Fund users
vm.deal(SELLER, STARTING_BALANCE);
vm.deal(BIDDER_1, STARTING_BALANCE);
vm.deal(BIDDER_2, STARTING_BALANCE);
}

Please add the following test to test/BidBeastsMarketPlaceTest.t.sol:BidBeastsNFTMarketTest:

function testAnyoneCanBurnAnyNFT() public {
_mintNFT();
assertEq(nft.ownerOf(TOKEN_ID), SELLER);
vm.prank(BIDDER_1);
nft.burn(TOKEN_ID);
vm.expectRevert(abi.encodeWithSelector(IERC721Errors.ERC721NonexistentToken.selector, TOKEN_ID));
address NFTowner = nft.ownerOf(TOKEN_ID);
}

Then run: forge test --mt testAnyoneCanBurnAnyNFT

Output:

Ran 1 test for test/BidBeastsMarketPlaceTest.t.sol:BidBeastsNFTMarketTest
[PASS] testAnyoneCanBurnAnyNFT() (gas: 73156)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 758.55µs (70.77µs CPU time)

Recommended Mitigation

In src/BidBeasts_NFT_ERC721.sol: BidBeasts::burn(uint256 _tokenId):

function burn(uint256 _tokenId) public {
+ address owner = ownerOf(_tokenId);
+ if (!_isAuthorized(owner, msg.sender, _tokenId)) {
+ revert ERC721InsufficientApproval(msg.sender, _tokenId);
+ }
_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.