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); +}