From fd57c33e7aef4430bd41e2183de2696b6b1fd676 Mon Sep 17 00:00:00 2001 From: 0X-SquidSol Date: Wed, 26 Aug 2026 14:56:50 -0400 Subject: [PATCH] fix(security,#437): stop asset_admin seizing an asset's oracle_authority `handle_update_asset_authority` skips the current holder's consent check whenever `admin_signed`. #430 scoped the carve-out to the two insurance legs, so `ASSET_AUTH_ORACLE` still took the bypass: `asset_admin` could take an asset's `oracle_authority` from a holder that never signed, then reach the asset's price surface through `ConfigureAuthMark` (which switches oracle_mode, so the asset need not already be in auth-mark) and `PushAuthMark`. Measured on a live market: mark 100 -> 999_000_000. Same root cause as #414 and #416/#417, through the fourth leg. This also restores the model the repo already documents in README: "Non-burn transfers require both the current authority and the new key to sign." Adds ASSET_AUTH_ORACLE to the existing exclusion list. Deliberately NOT the blanket form (`admin_bypass_permitted = current_value == [0u8; 32]`): that reintroduces the brick review of #415 caught, because `backing_bucket_authority` is bound to the LP vault registry PDA, a PDA cannot sign, and the field would be welded permanently. Verified -- under the blanket form, backing_authority_rotation_is_allowed_when_no_lp_vault_registry_exists fails with Custom(8). Operational cost, stated plainly: a lost or compromised oracle key can now be contained (ASSET_ACTION_SHUTDOWN, available to marketauth or asset_admin) but not replaced, and for asset 0 not replaced at all, since ASSET_ACTION_RETIRE rejects index 0. That is the same property the already-shipped insurance legs have, so this extends an accepted trade-off rather than introducing a new one. A follow-up worth considering is giving RestartAssetOracle an oracle_authority parameter -- it is already asset_admin-gated and only reachable from RECOVERY, so it would restore eviction without reopening the live-market seizure closed here. TDD: both tests written first and watched to FAIL on unfixed main; the two existing insurance tests keep passing throughout, which is what makes the differential meaningful. v16_wrapper 223 passed / 19 failed against a baseline of 221 / 19 -- the failing set matches tests/KNOWN_FAILING.txt exactly. Fork suites all green: lp_vault deposit 10, create 5, redeem 27, admin 8, state 21, authority canary 5, adversarial 14. Refs dcccrypto/percolator-prog#437 Co-Authored-By: Claude Opus 5 (1M context) --- src/v16_program.rs | 13 ++++- tests/v16_wrapper.rs | 135 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+), 1 deletion(-) diff --git a/src/v16_program.rs b/src/v16_program.rs index e761cf3d2..31932b7a3 100644 --- a/src/v16_program.rs +++ b/src/v16_program.rs @@ -12388,9 +12388,20 @@ pub mod processor { // // Assigning an UNHELD role (current_value == 0) still uses the admin path — // there is no holder to defend, and bootstrapping must stay possible. + // #437: the bypass must not reach the oracle leg either. `oracle_authority` + // gates ConfigureAuthMark / ConfigureEwmaMark / ConfigureHybridOracle and the + // two mark pushers, so taking it from a non-consenting holder hands over the + // asset's price surface on a live market. Same root cause as #414 and + // #416/#417, reached through the oracle leg. + // + // Note the `current_value == [0u8; 32]` escape is unreachable for ORACLE in + // practice: every activation path writes a non-zero oracle_authority, and the + // zero-burn above is rejected for every kind except ASSET_AUTH_ADMIN. It is + // kept for uniformity with the insurance legs, not because the oracle role can + // be vacant. let admin_bypass_permitted = !matches!( kind, - ASSET_AUTH_INSURANCE | ASSET_AUTH_INSURANCE_OPERATOR + ASSET_AUTH_INSURANCE | ASSET_AUTH_INSURANCE_OPERATOR | ASSET_AUTH_ORACLE ) || current_value == [0u8; 32]; if !(admin_signed && admin_bypass_permitted) { expect_live_authority(¤t_value, current.key)?; diff --git a/tests/v16_wrapper.rs b/tests/v16_wrapper.rs index 63e9f5ef5..1f08aa78e 100644 --- a/tests/v16_wrapper.rs +++ b/tests/v16_wrapper.rs @@ -20676,3 +20676,138 @@ fn v16_wrapper_asset_admin_cannot_seize_insurance_operator_from_holder() { ); assert_err_and_market_unchanged(seized, &market, &held); } + +/// The `admin_signed` bypass must not reach `ASSET_AUTH_ORACLE`. +/// +/// `oracle_authority` gates `ConfigureAuthMark`, `ConfigureEwmaMark`, +/// `ConfigureHybridOracle` and both mark pushers, so an `asset_admin` able to take +/// it from a non-consenting holder gets the asset's price surface on a live +/// market. Same root cause as the backing-bucket (#414) and insurance (#416/#417) +/// reports, reached through the oracle leg, and the same shape the repo already +/// documents: "Non-burn transfers require both the current authority and the new +/// key to sign" (README). +/// +/// Written as a differential so a future carve-out that re-opens one leg is +/// visible: identical instruction, identical accounts, only `kind` varies. +#[test] +fn v16_wrapper_asset_admin_cannot_seize_oracle_authority_from_holder() { + for (label, kind) in [ + ("insurance", ASSET_AUTH_INSURANCE), + ("insurance_operator", ASSET_AUTH_INSURANCE_OPERATOR), + ("oracle", ASSET_AUTH_ORACLE), + ] { + let mut admin = signer().writable(); + let mut market = market_account(); + let mut holder = signer(); + let mut attacker = signer(); + let _mint = init_market(&mut admin, &mut market); + + // Legitimately hand the role to `holder`, who co-signs. + run_ix( + Instruction::UpdateAssetAuthority { + asset_index: 0, + kind, + new_pubkey: holder.key.to_bytes(), + }, + &mut [&mut admin, &mut holder, &mut market], + ) + .unwrap_or_else(|e| panic!("{label}: holder-consented grant must succeed: {e:?}")); + + let held = market.data.clone(); + + // `asset_admin` tries to take it back. `holder` does NOT sign. + let seized = run_ix( + Instruction::UpdateAssetAuthority { + asset_index: 0, + kind, + new_pubkey: attacker.key.to_bytes(), + }, + &mut [&mut admin, &mut attacker, &mut market], + ); + assert_err_and_market_unchanged(seized, &market, &held); + + let profile = state::read_asset_oracle_profile(&market.data, 0).unwrap(); + let still_held = match kind { + ASSET_AUTH_INSURANCE => profile.insurance_authority, + ASSET_AUTH_INSURANCE_OPERATOR => profile.insurance_operator, + _ => profile.oracle_authority, + }; + assert_eq!( + still_held, + holder.key.to_bytes(), + "{label}: the consenting holder must keep the role" + ); + } +} + +/// The price surface that the oracle leg protects: with the seizure refused, an +/// `asset_admin` that never held `oracle_authority` cannot reach the mark through +/// `ConfigureAuthMark` / `PushAuthMark`, and the asset's mark is unchanged. +#[test] +fn v16_wrapper_asset_admin_cannot_reach_the_mark_without_the_oracle_authority() { + let mut admin = signer().writable(); + let mut market = market_account(); + let mut holder = signer(); + let mut attacker = signer().writable(); + let _mint = init_market(&mut admin, &mut market); + + run_ix( + Instruction::UpdateAssetAuthority { + asset_index: 0, + kind: ASSET_AUTH_ORACLE, + new_pubkey: holder.key.to_bytes(), + }, + &mut [&mut admin, &mut holder, &mut market], + ) + .expect("consented grant to holder"); + + let mark_before = state::read_asset_oracle_profile(&market.data, 0) + .unwrap() + .mark_ewma_e6; + let held = market.data.clone(); + + // Seizure is refused, so `attacker` never becomes the oracle authority. + let seized = run_ix( + Instruction::UpdateAssetAuthority { + asset_index: 0, + kind: ASSET_AUTH_ORACLE, + new_pubkey: attacker.key.to_bytes(), + }, + &mut [&mut admin, &mut attacker, &mut market], + ); + assert_err_and_market_unchanged(seized, &market, &held); + + // And the two instructions that write the mark stay closed to it. + assert!( + run_ix( + Instruction::ConfigureAuthMark { + asset_index: 0, + now_slot: 5, + initial_mark_e6: 1_000_000, + }, + &mut [&mut attacker, &mut market], + ) + .is_err(), + "ConfigureAuthMark must reject a non-holder" + ); + assert!( + run_ix( + Instruction::PushAuthMark { + asset_index: 0, + now_slot: 6, + mark_e6: 999_000_000, + }, + &mut [&mut attacker, &mut market], + ) + .is_err(), + "PushAuthMark must reject a non-holder" + ); + + let after = state::read_asset_oracle_profile(&market.data, 0).unwrap(); + assert_eq!(after.mark_ewma_e6, mark_before, "the asset's mark is untouched"); + assert_eq!( + after.oracle_authority, + holder.key.to_bytes(), + "the consenting holder still owns the oracle leg" + ); +}