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

Unauthorized token burning.

gridgirl

Missing access control on the burn function:

Permanent loss of assets(NFTs) for any token holder

Description

  • The standard behavior for an ERC-721 token's burn function is to allow only the token owner (or an approved party) to destroy the NFT.

  • The contract's burn function is declared as public with no explicit access control checks applied to verify the caller's ownership or approval status before token destruction. This specific issue allows any external address to successfully call burn with any valid tokenId, leading to the unauthorized and permanent destruction of tokens owned by others.


//The root cause is the lack of any access modifier or ownership check on the burn function:
function burn(uint256 _tokenId) public {
_burn(_tokenId); @> // The function is public and executes destruction without checking msg.sender's authority over _tokenId.
emit BidBeastsBurn(msg.sender, _tokenId);
}

Risk

Likelihood:

  • Reason 1: Direct Execution: An attacker knows the contract address and can directly call the burn(uint256) function on the blockchain using tools like Etherscan, regardless of any official dApp's authentication.

  • Reason 2: Zero-Cost Attack: The attacker does not need to own any tokens or possess any special privileges to execute the attack, only pay the transaction gas fee.

Impact:

  • Impact 1 - Permanent Asset Loss: Token owners suffer a permanent, unrecoverable loss of their NFTs, as the tokens are destroyed (sent to the zero address).

  • Impact 2 - Reputation Damage: The project and contract creator face severe reputational damage and potential legal consequences due to the loss of user assets.

Proof of Concept

The critical issue is the missing access control check within the public wrapper function burn(uint256 _tokenId). Although the inherited OpenZeppelin _burn function includes the check, the custom wrapper is redundant and potentially misleading. More importantly, we demonstrate that a non-owner can successfully call this function and destroy someone else's asset
​
contract UnauthorizedBurnPoC is Test {
// Define Actors
address public OWNER = vm.addr(999);
address public ALICE = vm.addr(101); // The victim
address public BOB = vm.addr(202); // The attacker
// Vulnerability setup (Alice owns token 0)
vm.prank(OWNER);
nftContract.mint(ALICE);
​
// --- THE CORE EXPLOIT ---
vm.startPrank(BOB);
nftContract.burn(0); // This is the line that succeeds due to missing access control.
vm.stopPrank();
​
// Verification of the impact (token is gone)
vm.expectRevert("ERC721: invalid token ID");
nftContract.ownerOf(0);

Recommended Mitigation

- remove this code
​
function burn(uint256 _tokenId) public {
//function declaration is vulnerable because it lacks the onlyOwner or a similar access control modifier.
_burn(_tokenId);
//When called from an unrestricted public function, this line executes the destruction of the token without verifying if msg.sender is the owner or an approved operator.
emit BidBeastsBurn(msg.sender, _tokenId);
}
//This custom event is redundant. The ERC721's internal _burn function already emits the standard Transfer event (to the zero address), which is the standard way to log a burn operation.
​
+ add this code
// CRITICAL SECURITY FIX: Added authorization check to prevent anyone from burning any token
function burn(uint256 _tokenId) public {
// This line ensures only the token owner or approved addresses can burn the token
// Without this, ANY address could burn ANY token in the collection
require(_isApprovedOrOwner(msg.sender, _tokenId), "Caller is not owner nor approved");
_burn(_tokenId); // Fixed: was *burn(*tokenId)
emit BidBeastsBurn(msg.sender, _tokenId);
}
}
​
Explanation
​
The Subtle Security Clarification (Access Control)
In your specific contract, the vulnerability was not that the $\_burn$ function lacked a check, but that your custom public wrapper function was a potential security trap and contained redundancy.
The vulnerability was described as "Unauthorized token burning" because a custom `public` function was created that simply called $\_burn$. While the $\text{OpenZeppelin}$ implementation of $\_burn$ does include the ownership check (`require(_isApprovedOrOwner(msg.sender, tokenId))`), using a custom wrapper function like this:
​
1. Increases Risk: If you were using an older $\text{ERC721}$ standard or a slightly different library, your custom `public` function would be vulnerable if it didn't explicitly add the check. The fix removes this potential risk of manual error.
2. Hides Intent: The custom function obscured the fact that the contract could rely purely on the inherited, secure mechanism.
​
The suggested fix confirms the secure implementation by showing the minimal, most standard, and gas-efficient approach to performing a secure burn in an $\text{OpenZeppelin}$ contract. **The core $\_burn(tokenId)$ call in the fix now stands alone, making it clear that all necessary security (access control) is inherited and functional.**
​
Updates

Lead Judging Commences

cryptoghost Lead Judge 2 months 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.

Give us feedback!