fix(contract): close open-proxy hole in burn (#1021) - #1059
Merged
Conversation
burn(token_address, from, amount) previously only checked the burn_enabled flag when the token happened to be registered (`if let Some(index)`), and looked the token up only *after* calling `balance` on it. For any address the factory never deployed this meant: - the burn_enabled invariant did not apply (unknown tokens bypassed it); - the factory would invoke an arbitrary external contract (running attacker code with the factory as caller) and emit its own official `burn` event referencing a token it never created, polluting the indexed history the Transaction History view renders. Make the TokenIndex lookup mandatory and move it *before* any external call: an unregistered address is now rejected with TokenNotFound without the factory ever touching it, and the burn_enabled gate is unconditional as a side effect. Holders of external tokens can still burn directly on those tokens' own contracts. mint_tokens, set_metadata and set_burn_enabled already gate on the per-token owner/index and reject unknown tokens; add explicit tests locking that boundary in so a future refactor cannot silently reopen it. Tests: burn on a never-registered address fails; a registered index with a missing TokenInfo fails; a factory burn event is only ever emitted for a factory token (and none for a rejected unregistered/disabled token); burn_enabled=false blocks burn with no bypass. Docs: document the trust boundary and TokenNotFound on burn in contract-abi.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
🎉 This PR is included in version 1.3.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
burn(token_address, from, amount)acted as an open proxy for arbitrary token contracts and skipped theburn_enabledgate for unknown tokens.The old code looked the token up only after calling
balanceon it, and gated onburn_enabledonly inside anif let Some(index)— so for any address the factory never deployed:burn_enabledinvariant did not apply (unknown tokens bypassed it); andburnevent referencing a token it never created — polluting the indexed history the Transaction History view renders.Fix
Make the
TokenIndexlookup mandatory and move it before any external call:token_addressis rejected withTokenNotFoundwithout the factory ever invoking it;burn_enabledgate is now unconditional (no code path reaches the burn call without verifying the flag).Holders of external (non-factory) tokens can still burn directly on those tokens' own contracts — there is no legitimate factory-burn path for them.
Trust-boundary audit
mint_tokens,set_metadata, andset_burn_enabledalready resolve the per-tokenowner/TokenIndexand reject unknown tokens (TokenNotFound) before doing anything. Added explicit tests locking that in so a future refactor can't silently reopen the boundary.Tests (7 new, all green — 165 total)
test_burn_unregistered_token_fails— burn on a never-registered address →TokenNotFound, no factory event, external balance untouched.test_burn_registered_index_missing_info_fails— dangling index →TokenNotFound.test_burn_event_only_emitted_for_factory_token— a factory burn event is emitted only for a factory token; a rejected unregistered attempt emits none.test_burn_disabled_no_bypass—burn_enabled=falseblocks burn with no bypass; balance untouched, no event.test_mint_tokens_unregistered_token_fails,test_set_metadata_unregistered_token_fails,test_set_burn_enabled_unregistered_token_fails— trust-boundary lock-in.cargo test(165 passed),cargo clippy -- -D warnings(clean),cargo fmt --check(clean).Acceptance criteria
burn_enabledis false, and no factory burn event can reference a non-factory token.Docs
docs/contract-abi.md— documented the trust boundary andTokenNotFoundonburn.Closes #1021