From 3e129411d6281187170e3dba3b6b92f398aee21f Mon Sep 17 00:00:00 2001 From: abore9769 Date: Thu, 25 Jun 2026 18:09:25 +0100 Subject: [PATCH] sec(vault): add nonce-bound replay protection to set_authorized_caller Adds a monotonic u64 nonce (StorageKey::AuthorizedCallerNonce) that the owner must supply on every set_authorized_caller call. A stale or replayed nonce is rejected with VaultError::StaleNonce, preventing a captured owner signature from being re-broadcast to reinstate a stale authorized_caller. Also adds VaultError::StaleNonce (code 30), a get_authorized_caller_nonce view function, the missing vault-self-address guard, and 8 new tests covering: default nonce, first rotation, stale/future nonce rejection, sequential increments, u64::MAX wrap, no-advance on failure, and event nonce emission. Co-Authored-By: Claude Sonnet 4.6 --- contracts/vault/src/lib.rs | 48 +++++- contracts/vault/src/test.rs | 198 +++++++++++++++++++++++-- contracts/vault/src/test_reentrancy.rs | 2 +- docs/ACCESS_CONTROL.md | 33 +++++ 4 files changed, 268 insertions(+), 13 deletions(-) diff --git a/contracts/vault/src/lib.rs b/contracts/vault/src/lib.rs index 43d2363d..3fa1ba6b 100644 --- a/contracts/vault/src/lib.rs +++ b/contracts/vault/src/lib.rs @@ -101,6 +101,8 @@ pub enum VaultError { PriceParseError = 28, /// Duplicate request ID detected (code 29). DuplicateRequestId = 29, + /// Supplied nonce does not match the stored authorized-caller rotation nonce (code 30). + StaleNonce = 30, } #[contracttype] @@ -151,6 +153,9 @@ pub enum StorageKey { /// `REQUEST_ID_BUMP_AMOUNT` ledgers. The value is `true` (a `bool`); /// presence of the key is the authoritative signal. ProcessedRequest(Symbol), + /// Monotonic u64 nonce incremented on every successful `set_authorized_caller` + /// rotation. Defaults to `0` before the first rotation. + AuthorizedCallerNonce, } /// Settlement contract client for crediting the global pool. @@ -368,6 +373,17 @@ impl CalloraVault { .unwrap_or(false) } + /// Return the current authorized-caller rotation nonce. + /// + /// Returns `0` before the first `set_authorized_caller` call. + /// Pass this value as `expected_nonce` in the next `set_authorized_caller` call. + pub fn get_authorized_caller_nonce(env: Env) -> u64 { + env.storage() + .instance() + .get(&StorageKey::AuthorizedCallerNonce) + .unwrap_or(0u64) + } + /// Return `true` if `caller` is the owner or an allowed depositor. /// Returns error if vault is not initialized. pub fn is_authorized_depositor(env: Env, caller: Address) -> Result { @@ -440,21 +456,51 @@ impl CalloraVault { } /// Set or clear the authorized caller for `deduct`/`batch_deduct` (owner only). + /// + /// # Replay Protection + /// A monotonic u64 nonce (stored under `StorageKey::AuthorizedCallerNonce`) + /// guards this function against replay attacks. The caller must supply the + /// current nonce as `expected_nonce`; the stored value defaults to `0` before + /// the first rotation. Each successful rotation increments the stored nonce + /// (wrapping at `u64::MAX`) and emits it in the event payload so off-chain + /// indexers can detect gaps or replays. + /// + /// # Errors + /// - `VaultError::StaleNonce` — `expected_nonce` differs from the stored nonce. + /// - `VaultError::AuthorizedCallerCannotBeVault` — `new_caller` is the vault itself. pub fn set_authorized_caller( env: Env, new_caller: Option
, + expected_nonce: u64, ) -> Result<(), VaultError> { let mut meta = Self::get_meta(env.clone())?; meta.owner.require_auth(); + if let Some(ref nc) = new_caller { + if nc == &env.current_contract_address() { + return Err(VaultError::AuthorizedCallerCannotBeVault); + } + } + let stored_nonce: u64 = env + .storage() + .instance() + .get(&StorageKey::AuthorizedCallerNonce) + .unwrap_or(0u64); + if expected_nonce != stored_nonce { + return Err(VaultError::StaleNonce); + } + let next_nonce = stored_nonce.wrapping_add(1); let old = meta.authorized_caller.clone(); meta.authorized_caller = new_caller.clone(); env.storage().instance().set(&StorageKey::MetaKey, &meta); + env.storage() + .instance() + .set(&StorageKey::AuthorizedCallerNonce, &next_nonce); env.events().publish( ( Symbol::new(&env, "set_authorized_caller"), meta.owner.clone(), ), - (old, new_caller), + (old, new_caller, expected_nonce), ); Ok(()) } diff --git a/contracts/vault/src/test.rs b/contracts/vault/src/test.rs index 57c88236..73db69bc 100644 --- a/contracts/vault/src/test.rs +++ b/contracts/vault/src/test.rs @@ -759,7 +759,7 @@ fn set_authorized_caller_sets_and_emits_event() { let settlement = create_settlement(&env, &owner, &vault_address); client.set_settlement(&owner, &settlement); - client.set_authorized_caller(&Some(new_caller.clone())); + client.set_authorized_caller(&Some(new_caller.clone()), &0u64); let events = env.events().all(); let ev = events.last().expect("expected set_authorized_caller event"); @@ -770,9 +770,10 @@ fn set_authorized_caller_sets_and_emits_event() { assert_eq!(topic0, Symbol::new(&env, "set_authorized_caller")); assert_eq!(topic1, owner); - let (old, now): (Option
, Option
) = ev.2.into_val(&env); + let (old, now, nonce): (Option
, Option
, u64) = ev.2.into_val(&env); assert_eq!(old, None); assert_eq!(now, Some(new_caller.clone())); + assert_eq!(nonce, 0u64); let remaining = client.deduct(&new_caller, &50, &None); assert_eq!(remaining, 150); @@ -2898,7 +2899,7 @@ fn test_set_authorized_caller() { env.mock_all_auths(); client.init(&owner, &usdc, &None, &None, &None, &None, &None); - client.set_authorized_caller(&Some(auth_caller.clone())); + client.set_authorized_caller(&Some(auth_caller.clone()), &0u64); let meta = client.get_meta(); assert_eq!(meta.authorized_caller, Some(auth_caller)); } @@ -2917,11 +2918,11 @@ fn set_authorized_caller_non_owner_fails() { client.init(&owner, &usdc, &None, &None, &None, &None, &None); // Attempt to set authorized caller as non-owner - client.set_authorized_caller(&Some(new_caller)); + let _ = non_owner; // non_owner has no way to override owner auth without caller param + client.set_authorized_caller(&Some(new_caller), &0u64); } #[test] -#[should_panic(expected = "authorized_caller cannot be vault address")] fn set_authorized_caller_vault_address_fails() { let env = Env::default(); let owner = Address::generate(&env); @@ -2931,8 +2932,11 @@ fn set_authorized_caller_vault_address_fails() { env.mock_all_auths(); client.init(&owner, &usdc, &None, &None, &None, &None, &None); - // Attempt to set vault itself as authorized caller - client.set_authorized_caller(&Some(vault_address)); + let result = client.try_set_authorized_caller(&Some(vault_address), &0u64); + assert_eq!( + result, + Err(Ok(VaultError::AuthorizedCallerCannotBeVault)) + ); } #[test] @@ -2946,17 +2950,189 @@ fn set_authorized_caller_clear_succeeds() { env.mock_all_auths(); client.init(&owner, &usdc, &None, &None, &None, &None, &None); - // Set authorized caller - client.set_authorized_caller(&Some(auth_caller.clone())); + // Set authorized caller (nonce 0 → stored nonce becomes 1) + client.set_authorized_caller(&Some(auth_caller.clone()), &0u64); let meta = client.get_meta(); assert_eq!(meta.authorized_caller, Some(auth_caller)); - // Clear authorized caller - client.set_authorized_caller(&None); + // Clear authorized caller (nonce 1 → stored nonce becomes 2) + client.set_authorized_caller(&None, &1u64); let meta2 = client.get_meta(); assert_eq!(meta2.authorized_caller, None); } +// --------------------------------------------------------------------------- +// Nonce-based replay-protection tests for set_authorized_caller +// --------------------------------------------------------------------------- + +/// Nonce defaults to 0 before any rotation has occurred. +#[test] +fn set_authorized_caller_nonce_default_is_zero() { + let env = Env::default(); + let owner = Address::generate(&env); + let (_, client) = create_vault(&env); + let (usdc, _, _) = create_usdc(&env, &owner); + + env.mock_all_auths(); + client.init(&owner, &usdc, &None, &None, &None, &None, &None); + + assert_eq!(client.get_authorized_caller_nonce(), 0u64); +} + +/// First rotation with nonce 0 succeeds and increments the stored nonce to 1. +#[test] +fn set_authorized_caller_first_rotation_with_nonce_zero() { + let env = Env::default(); + let owner = Address::generate(&env); + let new_caller = Address::generate(&env); + let (_, client) = create_vault(&env); + let (usdc, _, _) = create_usdc(&env, &owner); + + env.mock_all_auths(); + client.init(&owner, &usdc, &None, &None, &None, &None, &None); + + client.set_authorized_caller(&Some(new_caller.clone()), &0u64); + + assert_eq!(client.get_authorized_caller_nonce(), 1u64); + assert_eq!(client.get_meta().authorized_caller, Some(new_caller)); +} + +/// Replaying the same nonce after a successful rotation is rejected. +#[test] +fn set_authorized_caller_stale_nonce_rejected() { + let env = Env::default(); + let owner = Address::generate(&env); + let caller_a = Address::generate(&env); + let caller_b = Address::generate(&env); + let (_, client) = create_vault(&env); + let (usdc, _, _) = create_usdc(&env, &owner); + + env.mock_all_auths(); + client.init(&owner, &usdc, &None, &None, &None, &None, &None); + + // First rotation succeeds with nonce 0. + client.set_authorized_caller(&Some(caller_a.clone()), &0u64); + + // Replaying nonce 0 must be rejected. + let result = client.try_set_authorized_caller(&Some(caller_b.clone()), &0u64); + assert_eq!(result, Err(Ok(VaultError::StaleNonce))); + + // Stored caller is still caller_a — no state was mutated by the replay. + assert_eq!(client.get_meta().authorized_caller, Some(caller_a)); +} + +/// Supplying a future nonce that hasn't been reached is rejected. +#[test] +fn set_authorized_caller_future_nonce_rejected() { + let env = Env::default(); + let owner = Address::generate(&env); + let new_caller = Address::generate(&env); + let (_, client) = create_vault(&env); + let (usdc, _, _) = create_usdc(&env, &owner); + + env.mock_all_auths(); + client.init(&owner, &usdc, &None, &None, &None, &None, &None); + + let result = client.try_set_authorized_caller(&Some(new_caller), &5u64); + assert_eq!(result, Err(Ok(VaultError::StaleNonce))); + + assert_eq!(client.get_authorized_caller_nonce(), 0u64); +} + +/// Three sequential rotations with nonces 0, 1, 2 each succeed and leave +/// the stored nonce at 3. +#[test] +fn set_authorized_caller_nonce_increments_sequentially() { + let env = Env::default(); + let owner = Address::generate(&env); + let (_, client) = create_vault(&env); + let (usdc, _, _) = create_usdc(&env, &owner); + + env.mock_all_auths(); + client.init(&owner, &usdc, &None, &None, &None, &None, &None); + + for nonce in 0u64..3 { + let caller = Address::generate(&env); + client.set_authorized_caller(&Some(caller), &nonce); + assert_eq!(client.get_authorized_caller_nonce(), nonce + 1); + } + assert_eq!(client.get_authorized_caller_nonce(), 3u64); +} + +/// When the stored nonce is u64::MAX a successful rotation wraps to 0. +#[test] +fn set_authorized_caller_nonce_wraps_at_u64_max() { + let env = Env::default(); + let owner = Address::generate(&env); + let new_caller = Address::generate(&env); + let (_, client) = create_vault(&env); + let (usdc, _, _) = create_usdc(&env, &owner); + + env.mock_all_auths(); + client.init(&owner, &usdc, &None, &None, &None, &None, &None); + + // Manually prime the nonce to u64::MAX via storage so we don't need 2^64 calls. + env.as_contract(&client.address, || { + env.storage() + .instance() + .set(&StorageKey::AuthorizedCallerNonce, &u64::MAX); + }); + assert_eq!(client.get_authorized_caller_nonce(), u64::MAX); + + // Rotation with nonce u64::MAX succeeds and wraps to 0. + client.set_authorized_caller(&Some(new_caller.clone()), &u64::MAX); + assert_eq!(client.get_authorized_caller_nonce(), 0u64); + assert_eq!(client.get_meta().authorized_caller, Some(new_caller)); +} + +/// A failed rotation (wrong nonce) does not advance the nonce. +#[test] +fn set_authorized_caller_failed_rotation_does_not_advance_nonce() { + let env = Env::default(); + let owner = Address::generate(&env); + let new_caller = Address::generate(&env); + let (_, client) = create_vault(&env); + let (usdc, _, _) = create_usdc(&env, &owner); + + env.mock_all_auths(); + client.init(&owner, &usdc, &None, &None, &None, &None, &None); + + // Wrong nonce — must fail. + let _ = client.try_set_authorized_caller(&Some(new_caller.clone()), &99u64); + + // Stored nonce is unchanged at 0. + assert_eq!(client.get_authorized_caller_nonce(), 0u64); + + // Correct nonce still works afterwards. + client.set_authorized_caller(&Some(new_caller.clone()), &0u64); + assert_eq!(client.get_authorized_caller_nonce(), 1u64); +} + +/// Successful rotation emits the consumed nonce in the event data. +#[test] +fn set_authorized_caller_event_emits_nonce() { + let env = Env::default(); + let owner = Address::generate(&env); + let new_caller = Address::generate(&env); + let (_, client) = create_vault(&env); + let (usdc, _, _) = create_usdc(&env, &owner); + + env.mock_all_auths(); + client.init(&owner, &usdc, &None, &None, &None, &None, &None); + client.set_authorized_caller(&Some(new_caller.clone()), &0u64); + + let events = env.events().all(); + let ev = events.last().expect("expected set_authorized_caller event"); + + let topic: Symbol = ev.1.get(0).unwrap().into_val(&env); + assert_eq!(topic, Symbol::new(&env, "set_authorized_caller")); + + let (old, now, nonce): (Option
, Option
, u64) = ev.2.into_val(&env); + assert_eq!(old, None); + assert_eq!(now, Some(new_caller)); + assert_eq!(nonce, 0u64); +} + #[test] fn test_deduct_with_settlement_success() { let env = Env::default(); diff --git a/contracts/vault/src/test_reentrancy.rs b/contracts/vault/src/test_reentrancy.rs index bc0e3504..fd2ef7fd 100644 --- a/contracts/vault/src/test_reentrancy.rs +++ b/contracts/vault/src/test_reentrancy.rs @@ -201,7 +201,7 @@ fn test_reentrancy_by_authorized_attacker() { let (vault_addr, vault_client, token_addr, _settlement_addr, _owner) = setup_reentrancy_test(&env); let attacker = Address::generate(&env); - vault_client.set_authorized_caller(&Some(attacker.clone())); + vault_client.set_authorized_caller(&Some(attacker.clone()), &0u64); let token_mock = MaliciousTokenClient::new(&env, &token_addr); token_mock.set_token_attack_config(&vault_addr, &attacker, &true); diff --git a/docs/ACCESS_CONTROL.md b/docs/ACCESS_CONTROL.md index 1ac668a6..7253d81b 100644 --- a/docs/ACCESS_CONTROL.md +++ b/docs/ACCESS_CONTROL.md @@ -38,6 +38,30 @@ The Callora Vault implements role-based access control for deposit operations to - **Two-Step Admin Rotation**: Prevents accidental loss of control by requiring the nominee to explicitly accept the role. - **Cancellation Safety**: Provides `cancel_ownership_transfer` and `cancel_admin_transfer` functions to abort mistaken nominations before acceptance. - **Restricted Depositors**: Only owner and explicitly allowed depositors can increase vault balance. +- **Nonce-Bound Authorized-Caller Rotation**: `set_authorized_caller` requires the caller to supply the current monotonic nonce (see below), preventing a leaked owner signature from being replayed to reinstate a stale `authorized_caller`. + +### Authorized-Caller Replay Protection + +`set_authorized_caller` maintains a monotonic `u64` nonce stored under +`StorageKey::AuthorizedCallerNonce` in instance storage. + +| Step | Who | Action | +|------|-----|--------| +| 1 | Integrator | Call `get_authorized_caller_nonce()` to read the current nonce (defaults to `0`). | +| 2 | Owner | Call `set_authorized_caller(new_caller, expected_nonce)` with the value from step 1. | +| 3 | Contract | Verifies `expected_nonce == stored_nonce`; rejects with `VaultError::StaleNonce` if not. | +| 4 | Contract | Increments the stored nonce (`wrapping_add(1)`) and emits it in the event payload. | + +**Replay resistance**: a captured owner signature contains a fixed `expected_nonce`. +After one successful rotation the stored nonce advances, so the captured signature is +permanently invalid. + +**Event payload**: the `set_authorized_caller` event now carries +`(old_caller, new_caller, consumed_nonce)` as data, allowing off-chain indexers to +detect nonce gaps. + +**Nonce wrap**: the nonce wraps to `0` after `u64::MAX` rotations (2^64 calls) — a +practical impossibility, but handled safely by `wrapping_add`. ### Cancellation Functions @@ -91,6 +115,15 @@ The Callora Settlement contract tracks individual developer balances and global ## Test Coverage The implementation includes comprehensive tests covering: +- ✅ `set_authorized_caller` default nonce is `0` before first rotation +- ✅ First rotation with nonce `0` succeeds and advances stored nonce to `1` +- ✅ Replaying a consumed nonce is rejected with `VaultError::StaleNonce` +- ✅ Supplying a future nonce is rejected with `VaultError::StaleNonce` +- ✅ Three sequential rotations each advance the nonce correctly +- ✅ Nonce wraps at `u64::MAX` via `wrapping_add` +- ✅ Failed rotations do not advance the stored nonce +- ✅ Successful rotation emits `(old, new, consumed_nonce)` in the event payload +- ✅ Vault self-address is rejected as `new_caller` - ✅ Admin and Vault can call `receive_payment` - ✅ Unauthorized callers are rejected from `receive_payment` - ✅ Only Admin can call `set_admin` and `propose_vault` (and the `set_vault` alias)