From 6d3b414473c91fa6d28a76afca09169f489cd7ef Mon Sep 17 00:00:00 2001 From: real-venus Date: Thu, 25 Jun 2026 06:03:52 -0700 Subject: [PATCH] test(resource-token): prove mint/burn reject unauthorized calls (#4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The issue describes a #[requires_role] proc-macro auth bypass. No such macro (or Role enum) exists in this codebase; mint/burn are gated by authorize_mint/authorize_burn -> admin.require_auth(), which is sound and not bypassable. The real gap: every existing test uses env.mock_all_auths(), which auto-approves all authorization, so the gate is never exercised — a regression removing authorize_mint() from mint() would pass the whole suite. - test.rs: add negative-auth tests using env.set_auths(&[]) (empty auth set) proving mint and burn panic when the admin has not authorized, and that a rejected mint leaves balance and total_supply unchanged (try_mint -> Err) - docs/security/macro-auth.md: documents the actual authorization model and invariant, the scope correction (no macro exists), and a flagged follow-up: the operator-delegation path is not honored by mint/burn (admin-only), whose proper fix needs a breaking caller-Address signature change No contract behaviour changes; this hardens test coverage of the 'every mint/burn is authorized' invariant. --- contracts/docs/security/macro-auth.md | 78 +++++++++++++++++++++++++++ contracts/resource-token/src/test.rs | 70 ++++++++++++++++++++++++ 2 files changed, 148 insertions(+) create mode 100644 contracts/docs/security/macro-auth.md diff --git a/contracts/docs/security/macro-auth.md b/contracts/docs/security/macro-auth.md new file mode 100644 index 0000000..3dfc3c0 --- /dev/null +++ b/contracts/docs/security/macro-auth.md @@ -0,0 +1,78 @@ +# Mint/Burn Authorization — Model & Safety Invariants + +Issue #4 — "Custom Validation Macro Override in Resource Tokenization +Authorization" + +## Scope correction + +The issue describes a procedural macro `#[requires_role(Role::Minter)]` whose +expansion skips the authorization check when an `#[allow(unused)]` attribute is +present, allowing unauthorized minting. + +**No such macro exists in this codebase.** There is no `#[requires_role]` +attribute, no `Role` enum, and no `role_check` proc-macro crate. Authorization is +not attribute-driven, so the described macro-expansion bypass is not applicable. + +This document records the **actual** authorization model, the invariant it +upholds, and the tests that now lock it in. + +## Actual authorization model (`resource-token`) + +``` +mint(env, to, amount) -> authorize_mint(env) -> authorize_with_chain(env) +burn(env, from, amount) -> authorize_burn(env) -> authorize_with_chain(env) + +authorize_with_chain(env): + admin = get_admin(env) // panics "NoAdmin" if unset + admin.require_auth() // panics if the admin has not authorized +``` + +`authorize_*` is the **first statement** of `mint`/`burn`, before any state is +read or written. The gate is `Address::require_auth()`, which the Soroban host +validates against the actual authorization context — it cannot be spoofed by an +intermediate contract in the call chain. There is no code path that reaches the +balance/supply mutation without passing `admin.require_auth()`. + +### Invariant + +``` +∀ mint/burn operation: the admin has authorized the operation (require_auth) +``` + +## Why this could regress silently (the real gap the issue points at) + +Every pre-existing test uses `env.mock_all_auths()`, which auto-approves **all** +authorization. Under that mode the gate is never exercised: a regression that +deleted `authorize_mint()`/`authorize_burn()` from `mint`/`burn` would still pass +the entire suite. + +### Tests added + +`contracts/resource-token/src/test.rs`: + +- `test_mint_rejected_without_authorization` — after setup, drop all auth with + `env.set_auths(&[])`; `mint` must panic. +- `test_burn_rejected_without_authorization` — same, for `burn`. +- `test_mint_rejected_without_auth_leaves_state_unchanged` — `try_mint` returns + `Err` and neither balance nor total supply changes. + +These exercise the gate with an empty authorization set, so removing or bypassing +the authorization call now fails the suite. + +## Known limitation / recommended follow-up (out of scope here) + +`authorize_with_chain` only honors the **admin**. The contract also exposes +`authorize_operator` / `is_valid_operator` and documents that "the admin or a +valid operator can mint", but the mint/burn path **never checks operators** — an +operator cannot actually mint, because `require_auth()` is only ever called on +the admin's address. + +Making operator-delegated minting work securely requires threading the caller's +`Address` into `mint`/`burn` (so the contract can `require_auth()` the operator +and verify it is currently valid). That is a **breaking signature change** and is +intentionally left as a separate change; the misleading documentation should be +corrected at the same time. + +The current behaviour is **safe** (only the admin can mint/burn); the limitation +is reduced functionality, not an authorization bypass. +``` diff --git a/contracts/resource-token/src/test.rs b/contracts/resource-token/src/test.rs index 3020561..7767980 100644 --- a/contracts/resource-token/src/test.rs +++ b/contracts/resource-token/src/test.rs @@ -485,3 +485,73 @@ fn test_burn_after_max_supply_allows_reminting() { assert_eq!(client.total_supply(), MAX_SUPPLY); assert_eq!(client.balance(&recipient), MAX_SUPPLY); } + +// --- Mint/Burn authorization enforcement (issue #4) ---------------------- +// +// The contract gates mint()/burn() with authorize_mint()/authorize_burn(), +// which require the admin's authorization (admin.require_auth()). Every other +// test in this file uses `mock_all_auths()`, which auto-approves all auth and +// therefore never exercises the gate — a regression that removed the +// authorization call would pass all of those tests. These tests provide an +// EMPTY authorization set via `set_auths(&[])` so the gate is actually exercised +// and proven to reject unauthorized mint/burn (the invariant: every mint/burn +// must be authorized by the admin). + +#[test] +#[should_panic] +fn test_mint_rejected_without_authorization() { + let env = Env::default(); + let contract_id = env.register_contract(None, ResourceToken); + let client = ResourceTokenClient::new(&env, &contract_id); + + let admin = Address::generate(&env); + let recipient = Address::generate(&env); + + // Authorize only the setup, then drop all authorizations. + env.mock_all_auths(); + client.initialize(&admin); + env.set_auths(&[]); // no authorization provided for the next call + + // Must panic: the admin has not authorized this mint. + client.mint(&recipient, &1000); +} + +#[test] +#[should_panic] +fn test_burn_rejected_without_authorization() { + let env = Env::default(); + let contract_id = env.register_contract(None, ResourceToken); + let client = ResourceTokenClient::new(&env, &contract_id); + + let admin = Address::generate(&env); + let account = Address::generate(&env); + + env.mock_all_auths(); + client.initialize(&admin); + client.mint(&account, &1000); + env.set_auths(&[]); // no authorization provided for the next call + + // Must panic: the admin has not authorized this burn. + client.burn(&account, &500); +} + +#[test] +fn test_mint_rejected_without_auth_leaves_state_unchanged() { + // Belt-and-suspenders: a rejected mint must not move balance or supply. + let env = Env::default(); + let contract_id = env.register_contract(None, ResourceToken); + let client = ResourceTokenClient::new(&env, &contract_id); + + let admin = Address::generate(&env); + let recipient = Address::generate(&env); + + env.mock_all_auths(); + client.initialize(&admin); + env.set_auths(&[]); + + // try_* returns Err instead of unwinding, so we can assert on the aftermath. + let result = client.try_mint(&recipient, &1000); + assert!(result.is_err(), "unauthorized mint must be rejected"); + assert_eq!(client.balance(&recipient), 0); + assert_eq!(client.total_supply(), 0); +}