-
Notifications
You must be signed in to change notification settings - Fork 1.6k
NFTokenBurn::preclaim should explicitly reject burn when issuer account SLE is absent #6659
Copy link
Copy link
Open
Labels
AI TriageBugs and fixes that have been triaged via AI initiativesBugs and fixes that have been triaged via AI initiativesTechnical Debt
Description
Summary
In NFTokenBurn::preclaim, when a non-owner attempts to burn a burnable NFT and the issuer's account has been deleted (SLE is null), the permission check silently falls through to tesSUCCESS instead of explicitly rejecting the transaction.
Code
src/libxrpl/tx/transactors/nft/NFTokenBurn.cpp lines 35-42:
if (auto const issuer = nft::getIssuer(ctx.tx[sfNFTokenID]); issuer != account)
{
if (auto const sle = ctx.view.read(keylet::account(issuer)); sle)
{
if (auto const minter = (*sle)[~sfNFTokenMinter]; minter != account)
return tecNO_PERMISSION;
}
// ← if sle is null, falls through to tesSUCCESS
}Impact
This is currently unreachable because AccountDelete::preclaim blocks deletion when sfMintedNFTokens != sfBurnedNFTokens — so the issuer's SLE can never be null while any of their NFTs exist. No exploit is possible.
However, the defensive fix is trivial: return tecNO_PERMISSION (or tecINTERNAL) when sle is null, rather than silently succeeding.
Suggested Fix
Add an else branch:
if (auto const sle = ctx.view.read(keylet::account(issuer)); sle)
{
if (auto const minter = (*sle)[~sfNFTokenMinter]; minter != account)
return tecNO_PERMISSION;
}
else
{
return tecNO_PERMISSION;
}Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
AI TriageBugs and fixes that have been triaged via AI initiativesBugs and fixes that have been triaged via AI initiativesTechnical Debt