From a358483d3bb8646fc43b5b099f8c3c547bf9adc9 Mon Sep 17 00:00:00 2001 From: carrion256 Date: Tue, 5 May 2026 17:40:39 +0200 Subject: [PATCH 1/4] fix: harden Soroban storage lifecycle (#FIND-017 #FIND-015 #FIND-016 #FIND-036 #FIND-029 #FIND-033 #FIND-034 #FIND-035 Nexus 49a0ad1a-499b-4ace-b7b4-ceaca30a5cf3 Nexus 53cec8fe-3e21-4a50-83e2-67717301b300 Nexus 3966d571-d2e2-43ba-aff1-def6813725e5 Nexus e1461d5a-d2e4-4808-993f-f82d403aad43 Nexus 69a8fd3b-1e78-41e0-811c-9b25bfcb906a Nexus e7868ba6-70e0-4082-b2a9-5518e5180d4f Nexus be1dbcdd-98f0-4b20-bc58-3cd9ae684e6e Nexus 66b6e854-524e-4047-8c9d-503e06c8e470) Trace: 44401156a682c0ae7f4ff781c055ce16a684906e --- contract/vault/soroban/README.md | 9 +- contract/vault/soroban/STRIDE.md | 4 +- .../soroban/src/contract/curator_vault.rs | 2 +- .../vault/soroban/src/contract/entrypoints.rs | 3 +- .../vault/soroban/src/contract/helpers.rs | 58 +++++- contract/vault/soroban/src/contract/mod.rs | 5 +- contract/vault/soroban/src/fungible_vault.rs | 7 +- contract/vault/soroban/src/storage/mod.rs | 186 ++++++++++++++++-- contract/vault/soroban/src/tests.rs | 184 ++++++++++++++++- 9 files changed, 423 insertions(+), 35 deletions(-) diff --git a/contract/vault/soroban/README.md b/contract/vault/soroban/README.md index 5f2794673..ab3b0caa8 100644 --- a/contract/vault/soroban/README.md +++ b/contract/vault/soroban/README.md @@ -194,15 +194,16 @@ Useful commands: ## State Size and Operational Limits - Soroban enforces per-entry and per-transaction resource limits. Current network values are documented by Stellar: https://developers.stellar.org/docs/networks/resource-limits-fees -- Vault runtime state is persisted as a single `StateBlob`, so serialized `VaultState` size is the practical storage-pressure point. -- The main long-lived growth vector is pending withdrawals, which are bounded by `MAX_PENDING = 1024`. -- In-flight operation plans (`Allocating.plan`, `Refreshing.plan`) are expected to remain small under allocator policy, so the 1024 pending-withdrawal cap is the dominant operational bound in practice. +- Vault runtime state is persisted as a single versioned `StateBlob`, so serialized `VaultState` size is the practical storage-pressure point. +- The shared kernel still has an absolute `MAX_PENDING = 1024`, but Soroban uses the chain-specific `SOROBAN_MAX_PENDING_WITHDRAWALS = 512` runtime cap to stay below the 64 KiB contract-data-entry limit with room for schema growth. +- In-flight operation plans (`Allocating.plan`, `Refreshing.plan`) are expected to remain small under allocator policy, so the 512 Soroban pending-withdrawal cap is the dominant operational bound in practice. +- Persistent storage blobs carry a compact `TVS` version header on new writes. Decoders still accept the pre-header legacy layout so governed `migrate()` can validate and rewrite old blobs in place. ## Practical Risk Model - TVL growth by itself does not significantly increase serialized state size. - Risk comes from queue backlog plus unusually large in-flight plans. -- If state exceeds Soroban storage write limits, the transaction fails atomically (no partial state commit). +- If state would exceed Soroban storage write limits, storage save paths return a typed runtime storage error before the host storage write. ## Parity Tests diff --git a/contract/vault/soroban/STRIDE.md b/contract/vault/soroban/STRIDE.md index 161e1f2ad..9fb523bba 100644 --- a/contract/vault/soroban/STRIDE.md +++ b/contract/vault/soroban/STRIDE.md @@ -224,8 +224,8 @@ emit admin events via the existing `emit_admin_event()` pattern. | | **Information Disclosure** | **Info.1.R.1** — Accepted: no confidentiality assumptions on-chain. This is expected behavior for public blockchain contracts. **Info.1.R.2** — Avoid introducing unnecessary detailed event payloads that could leak operational patterns. | | | **Info.2.R.1** — Accepted risk: governance and fee configuration are operational parameters that need to be publicly verifiable. **Info.2.R.2** — Protect governance/curator keys through operational security (multisig, HSM), not obscurity. | | | **Info.3.R.1** — Accepted risk: queue transparency is a feature for user trust. **Info.3.R.2** — Monitor for unusual withdrawal patterns that might indicate front-running. **Info.3.R.3** — Consider: withdrawal cooldown (`DEFAULT_COOLDOWN_NS`) already provides some protection against same-block front-running. | -| **Denial of Service** | **DoS.1.R.1** — Accepted operational model: queued withdrawals require keeper/operator progression. Queue bounded by `MAX_PENDING = 1024`; guarded transitions prevent partial corruption. **DoS.1.R.2** — Operate redundant keepers and queue-staleness alerts. | -| | **DoS.2.R.1** — Ensure governance key/contract is resilient (multisig/DAO, signer redundancy) to avoid migration-state liveness failures. **DoS.2.R.2** — Test upgrade/migrate flow thoroughly on testnet before mainnet deployment. **DoS.2.R.3** — ✅ **Implemented**: `cancel_migration()` governance method added. Governance can cancel a pending migration, reverting the contract to operational state if `migrate()` has not been called. **DoS.2.R.4** — Document the upgrade procedure and key custody requirements. | +| **Denial of Service** | **DoS.1.R.1** — Accepted operational model: queued withdrawals require keeper/operator progression. Soroban runtime caps pending withdrawals at `SOROBAN_MAX_PENDING_WITHDRAWALS = 512` so the monolithic `StateBlob` stays below the 64 KiB contract-data-entry limit with schema-growth margin. **DoS.1.R.2** — Operate redundant keepers and queue-staleness alerts. | +| | **DoS.2.R.1** — Ensure governance key/contract is resilient (multisig/DAO, signer redundancy) to avoid migration-state liveness failures. **DoS.2.R.2** — Test upgrade/migrate flow thoroughly on testnet before mainnet deployment. **DoS.2.R.3** — ✅ **Implemented**: `cancel_migration()` governance method clears the migration gate and returns the contract to operational state, but it is not a WASM rollback primitive after `update_current_contract_wasm`; bad-code recovery still requires a follow-up governed upgrade/migration when chain execution permits it. **DoS.2.R.4** — Document the upgrade procedure and key custody requirements. | | | **DoS.3.R.1** — Adapter failures are localized to affected market operations; maintain diversified/vetted adapters. **DoS.3.R.2** — ✅ Implemented: adapter allowlist + queue-index routing enables rapid disabling of bad adapters. | | | **DoS.4.R.1** — Accepted with monitoring based on current sizing math and workload expectations. **DoS.4.R.2** — Keep telemetry on queue depth/resource usage and revisit if workload or network limits change. | | | **DoS.5.R.1** — `extend_ttl()` is permissionless — anyone can call it. TTL threshold is ~30 days (518,400 ledgers), extension is to ~6 months (3,110,400 ledgers). **DoS.5.R.2** — Operate a keeper bot that calls `extend_ttl` periodically. **DoS.5.R.3** — `save_state` and `save_address` automatically extend TTL on writes, providing additional safety margin. | diff --git a/contract/vault/soroban/src/contract/curator_vault.rs b/contract/vault/soroban/src/contract/curator_vault.rs index 04e24686c..39690eb78 100644 --- a/contract/vault/soroban/src/contract/curator_vault.rs +++ b/contract/vault/soroban/src/contract/curator_vault.rs @@ -166,7 +166,7 @@ where fees: self.config.fees, min_withdrawal_assets: MIN_WITHDRAWAL_ASSETS, withdrawal_cooldown_ns: DEFAULT_COOLDOWN_NS, - max_pending_withdrawals: MAX_PENDING as u32, + max_pending_withdrawals: SOROBAN_MAX_PENDING_WITHDRAWALS, paused: self.paused, virtual_shares: self.config.virtual_shares, virtual_assets: self.config.virtual_assets, diff --git a/contract/vault/soroban/src/contract/entrypoints.rs b/contract/vault/soroban/src/contract/entrypoints.rs index b723111ca..e1f8b2720 100644 --- a/contract/vault/soroban/src/contract/entrypoints.rs +++ b/contract/vault/soroban/src/contract/entrypoints.rs @@ -10,7 +10,7 @@ use super::helpers::{ load_virtual_offsets, migrate_legacy_paused, migration_in_progress, require_contract_address, require_governance, require_signed, sdk_string_to_alloc, set_config_address, set_migration_in_progress, store_fees_spec, store_virtual_offsets, - with_contract_vault_contract_error, + validate_and_rewrite_storage, with_contract_vault_contract_error, }; use super::*; use templar_soroban_shared_types::{ @@ -989,6 +989,7 @@ impl SorobanVaultContract { } migrate_legacy_paused(&env); + runtime_to_contract(validate_and_rewrite_storage(&env))?; extend_storage_ttl(&env); set_migration_in_progress(&env, false); emit_admin_event(&env, symbol_short!("migrate")); diff --git a/contract/vault/soroban/src/contract/helpers.rs b/contract/vault/soroban/src/contract/helpers.rs index e77dad929..eb430ad17 100644 --- a/contract/vault/soroban/src/contract/helpers.rs +++ b/contract/vault/soroban/src/contract/helpers.rs @@ -324,8 +324,8 @@ pub(crate) fn apply_fee_change( runtime_to_contract(store_fees_spec(env, &fees))?; let storage = SorobanStorage::new(env); - storage.save_address(&performance_kernel, &performance_recipient); - storage.save_address(&management_kernel, &management_recipient); + runtime_to_contract(storage.save_address(&performance_kernel, &performance_recipient))?; + runtime_to_contract(storage.save_address(&management_kernel, &management_recipient))?; Ok(()) } @@ -335,6 +335,29 @@ pub(crate) fn extend_storage_ttl(env: &Env) { .extend_ttl(DEFAULT_TTL_THRESHOLD, DEFAULT_TTL_EXTEND_TO); let storage = SorobanStorage::new(env); storage.extend_ttl(DEFAULT_TTL_THRESHOLD, DEFAULT_TTL_EXTEND_TO); + if let Ok(fees) = load_fees_spec(env) { + storage.extend_address_ttl( + &fees.performance.recipient, + DEFAULT_TTL_THRESHOLD, + DEFAULT_TTL_EXTEND_TO, + ); + storage.extend_address_ttl( + &fees.management.recipient, + DEFAULT_TTL_THRESHOLD, + DEFAULT_TTL_EXTEND_TO, + ); + } + for key in [ + VaultDataKey::Curator, + VaultDataKey::Governance, + VaultDataKey::AssetToken, + VaultDataKey::ShareToken, + ] { + if let Ok(address) = get_config_address(env, &key) { + let kernel = kernel_address_from_sdk(env, &address); + storage.extend_address_ttl(&kernel, DEFAULT_TTL_THRESHOLD, DEFAULT_TTL_EXTEND_TO); + } + } } pub(crate) fn get_config_address( @@ -367,6 +390,15 @@ pub(crate) fn sdk_string_to_alloc( } pub(crate) fn migrate_legacy_paused(env: &Env) { + if env + .storage() + .instance() + .get::<_, bool>(&LEGACY_PAUSED_MIGRATED_KEY) + .unwrap_or(false) + { + return; + } + let storage = SorobanStorage::new(env); if let Some(paused) = env @@ -376,12 +408,34 @@ pub(crate) fn migrate_legacy_paused(env: &Env) { { storage.set_paused(paused); env.storage().instance().remove(&VaultDataKey::Paused); + env.storage() + .instance() + .set(&LEGACY_PAUSED_MIGRATED_KEY, &true); return; } if let Some(paused) = storage.take_legacy_paused() { storage.set_paused(paused); } + env.storage() + .instance() + .set(&LEGACY_PAUSED_MIGRATED_KEY, &true); +} + +pub(crate) fn validate_and_rewrite_storage(env: &Env) -> Result<(), RuntimeError> { + let mut storage = SorobanStorage::new(env); + if let Some(state) = storage.load_state()? { + storage.save_state(&state)?; + } + if let Some(policy) = storage.load_policy_state()? { + storage.save_policy_state(&policy)?; + } + if let Some(restrictions) = Storage::load_restrictions(&storage)? { + Storage::save_restrictions(&mut storage, &Some(restrictions))?; + } + let fees = load_fees_spec(env)?; + store_fees_spec(env, &fees)?; + Ok(()) } #[inline(never)] diff --git a/contract/vault/soroban/src/contract/mod.rs b/contract/vault/soroban/src/contract/mod.rs index becb9342a..78e2224bf 100644 --- a/contract/vault/soroban/src/contract/mod.rs +++ b/contract/vault/soroban/src/contract/mod.rs @@ -50,12 +50,15 @@ use templar_vault_kernel::{ apply_action, convert_to_assets, convert_to_assets_ceil, convert_to_shares, convert_to_shares_ceil, plan_idle_payout, withdrawal_settled, Address, FeeAccrualAnchor, FeeSlot, FeesSpec, KernelAction, OpState, PayoutOutcome, Restrictions, TargetId, TimestampNs, - VaultConfig, VaultState, Wad, MAX_MANAGEMENT_FEE_WAD, MAX_PENDING, MAX_PERFORMANCE_FEE_WAD, + VaultConfig, VaultState, Wad, MAX_MANAGEMENT_FEE_WAD, MAX_PERFORMANCE_FEE_WAD, MIN_WITHDRAWAL_ASSETS, }; +use crate::storage::SOROBAN_MAX_PENDING_WITHDRAWALS; + pub(crate) const KERNEL_ADDRESS_DOMAIN: &[u8] = b"templar:soroban:address"; const MIGRATION_FLAG_KEY: soroban_sdk::Symbol = symbol_short!("migrate"); +pub(crate) const LEGACY_PAUSED_MIGRATED_KEY: soroban_sdk::Symbol = symbol_short!("pmigdn"); pub(crate) fn decode_command(payload: &Bytes) -> Result { VaultCommand::decode(&payload.to_alloc_vec()).map_err(|_| ContractError::InvalidInput) diff --git a/contract/vault/soroban/src/fungible_vault.rs b/contract/vault/soroban/src/fungible_vault.rs index 8ee194817..8c40a8ed3 100644 --- a/contract/vault/soroban/src/fungible_vault.rs +++ b/contract/vault/soroban/src/fungible_vault.rs @@ -17,14 +17,13 @@ use soroban_sdk::{token, Address as SdkAddress, Env}; use templar_vault_kernel::state::queue::DEFAULT_COOLDOWN_NS; use templar_vault_kernel::{ compute_fee_shares_from_assets, compute_management_fee_shares, total_assets_for_fee_accrual, - FeeAccrualAnchor, Number, TimestampNs, VaultConfig, VaultState, MAX_PENDING, - MIN_WITHDRAWAL_ASSETS, + FeeAccrualAnchor, Number, TimestampNs, VaultConfig, VaultState, MIN_WITHDRAWAL_ASSETS, }; use crate::contract::{load_fees_spec, load_virtual_offsets, VaultDataKey}; use crate::convert::{ledger_timestamp_ns, runtime_to_contract}; use crate::error::ContractError; -use crate::storage::{SorobanStorage, Storage}; +use crate::storage::{SorobanStorage, Storage, SOROBAN_MAX_PENDING_WITHDRAWALS}; fn preview_state_with_fee_accrual( env: &Env, @@ -88,7 +87,7 @@ pub(crate) fn load_state_and_config(env: &Env) -> Result<(VaultState, VaultConfi fees: runtime_to_contract(load_fees_spec(env))?, min_withdrawal_assets: MIN_WITHDRAWAL_ASSETS, withdrawal_cooldown_ns: DEFAULT_COOLDOWN_NS, - max_pending_withdrawals: MAX_PENDING as u32, + max_pending_withdrawals: SOROBAN_MAX_PENDING_WITHDRAWALS, paused: storage.is_paused(), virtual_shares, virtual_assets, diff --git a/contract/vault/soroban/src/storage/mod.rs b/contract/vault/soroban/src/storage/mod.rs index 57321761b..c32f0aa78 100644 --- a/contract/vault/soroban/src/storage/mod.rs +++ b/contract/vault/soroban/src/storage/mod.rs @@ -28,6 +28,19 @@ pub(crate) const DEFAULT_TTL_THRESHOLD: u32 = 518_400; /// For a vault contract holding real assets, maximum TTL prevents state /// loss during extended pauses or periods of inactivity. pub(crate) const DEFAULT_TTL_EXTEND_TO: u32 = 3_110_400; +pub(crate) const SOROBAN_MAX_PENDING_WITHDRAWALS: u32 = 512; +pub(crate) const SOROBAN_MAX_RESTRICTION_ADDRESSES: usize = 1024; + +const MAX_CONTRACT_DATA_ENTRY_SIZE_BYTES: usize = 64 * 1024; +const STORAGE_MAGIC: [u8; 3] = *b"TVS"; +const STORAGE_VERSION_CURRENT: u8 = 1; +const STORAGE_KIND_STATE: u8 = 1; +const STORAGE_KIND_RESTRICTIONS: u8 = 2; +const STORAGE_KIND_SUPPLY_QUEUE: u8 = 3; +const STORAGE_KIND_MARKETS: u8 = 4; +const STORAGE_KIND_PRINCIPALS: u8 = 5; +const STORAGE_KIND_CAP_GROUPS: u8 = 6; +const STORAGE_KIND_POLICY_LOCKS: u8 = 7; /// Internal persistent storage keys. Using Symbol constants instead of a /// `#[contracttype]` enum to avoid contractspec bloat and enum conversion codegen. @@ -47,6 +60,30 @@ impl SorobanStorageKey { pub const PausedState: Symbol = symbol_short!("paused_s"); } +fn push_storage_header(out: &mut Vec, kind: u8) { + out.extend_from_slice(&STORAGE_MAGIC); + push_u8(out, kind); + push_u8(out, STORAGE_VERSION_CURRENT); +} + +fn storage_payload(bytes: &[u8], kind: u8) -> Result<&[u8], RuntimeError> { + if bytes.len() >= 5 && bytes[..3] == STORAGE_MAGIC && bytes[3] == kind { + return match bytes[4] { + STORAGE_VERSION_CURRENT => Ok(&bytes[5..]), + _ => Err(RuntimeError::storage_error("unsupported storage version")), + }; + } + Ok(bytes) +} + +fn ensure_contract_data_entry_size(bytes: &[u8]) -> Result<(), RuntimeError> { + if bytes.len() <= MAX_CONTRACT_DATA_ENTRY_SIZE_BYTES { + Ok(()) + } else { + Err(RuntimeError::storage_error("persistent entry too large")) + } +} + fn push_u8(out: &mut Vec, value: u8) { out.push(value); } @@ -140,6 +177,7 @@ fn decode_cap_group_id(bytes: &[u8], cursor: &mut usize) -> Result Vec { let mut out = Vec::new(); + push_storage_header(&mut out, STORAGE_KIND_RESTRICTIONS); match mode { Restrictions::Blacklist(addresses) => { push_u8(&mut out, 0); @@ -160,9 +198,13 @@ pub(crate) fn encode_restrictions(mode: &Restrictions) -> Vec { } pub(crate) fn decode_restrictions(bytes: &[u8]) -> Result { + let bytes = storage_payload(bytes, STORAGE_KIND_RESTRICTIONS)?; let mut cursor = 0usize; let tag = read_u8(bytes, &mut cursor)?; let len = read_u32(bytes, &mut cursor)? as usize; + if len > SOROBAN_MAX_RESTRICTION_ADDRESSES { + return Err(RuntimeError::storage_error("restrictions too large")); + } let mut addresses = Vec::with_capacity(len); for _ in 0..len { addresses.push(read_address(bytes, &mut cursor)?); @@ -178,6 +220,7 @@ pub(crate) fn decode_restrictions(bytes: &[u8]) -> Result Vec { let mut out = Vec::new(); + push_storage_header(&mut out, STORAGE_KIND_SUPPLY_QUEUE); let max_length = queue.max_length().map(|value| value.get()).unwrap_or(0); push_u32(&mut out, max_length); let entries = queue.entries(); @@ -191,6 +234,7 @@ pub(crate) fn encode_supply_queue(queue: &SupplyQueue) -> Vec { } pub(crate) fn decode_supply_queue(bytes: &[u8]) -> Result { + let bytes = storage_payload(bytes, STORAGE_KIND_SUPPLY_QUEUE)?; let mut cursor = 0usize; let max_length = read_u32(bytes, &mut cursor)?; let count = read_u32(bytes, &mut cursor)? as usize; @@ -212,6 +256,7 @@ pub(crate) fn decode_supply_queue(bytes: &[u8]) -> Result) -> Vec { let mut out = Vec::new(); + push_storage_header(&mut out, STORAGE_KIND_CAP_GROUPS); push_u32(&mut out, cap_groups.len() as u32); for (id, record) in cap_groups.iter() { encode_cap_group_id(id, &mut out); @@ -237,6 +282,7 @@ pub(crate) fn encode_cap_groups(cap_groups: &OrderedMap Result, RuntimeError> { + let bytes = storage_payload(bytes, STORAGE_KIND_CAP_GROUPS)?; let mut cursor = 0usize; let count = read_u32(bytes, &mut cursor)? as usize; let mut cap_groups = OrderedMap::new(); @@ -272,6 +318,7 @@ pub(crate) fn decode_cap_groups( pub(crate) fn encode_markets(markets: &OrderedMap) -> Vec { let mut out = Vec::new(); + push_storage_header(&mut out, STORAGE_KIND_MARKETS); push_u32(&mut out, markets.len() as u32); for (target_id, config) in markets.iter() { push_u32(&mut out, *target_id); @@ -291,6 +338,7 @@ pub(crate) fn encode_markets(markets: &OrderedMap) -> Ve pub(crate) fn decode_markets( bytes: &[u8], ) -> Result, RuntimeError> { + let bytes = storage_payload(bytes, STORAGE_KIND_MARKETS)?; let mut cursor = 0usize; let count = read_u32(bytes, &mut cursor)? as usize; let mut markets = OrderedMap::new(); @@ -315,6 +363,7 @@ pub(crate) fn decode_markets( pub(crate) fn encode_principals(principals: &OrderedMap) -> Vec { let mut out = Vec::new(); + push_storage_header(&mut out, STORAGE_KIND_PRINCIPALS); push_u32(&mut out, principals.len() as u32); for (target_id, principal) in principals.iter() { push_u32(&mut out, *target_id); @@ -324,6 +373,7 @@ pub(crate) fn encode_principals(principals: &OrderedMap) -> Vec< } pub(crate) fn decode_principals(bytes: &[u8]) -> Result, RuntimeError> { + let bytes = storage_payload(bytes, STORAGE_KIND_PRINCIPALS)?; let mut cursor = 0usize; let count = read_u32(bytes, &mut cursor)? as usize; let mut principals = OrderedMap::new(); @@ -338,6 +388,7 @@ pub(crate) fn decode_principals(bytes: &[u8]) -> Result Vec { let mut out = Vec::new(); + push_storage_header(&mut out, STORAGE_KIND_POLICY_LOCKS); push_u32(&mut out, leases.stored_len() as u32); push_u64(&mut out, leases.next_fencing_token()); for (_, lease) in leases.iter() { @@ -358,6 +409,7 @@ pub(crate) fn encode_policy_locks(leases: &MarketLeaseRegistry) -> Vec { } pub(crate) fn decode_policy_locks(bytes: &[u8]) -> Result { + let bytes = storage_payload(bytes, STORAGE_KIND_POLICY_LOCKS)?; let mut cursor = 0usize; let count = read_u32(bytes, &mut cursor)? as usize; let next_fencing_token = read_u64(bytes, &mut cursor)?; @@ -414,9 +466,34 @@ fn decode_withdraw_queue(bytes: &[u8], cursor: &mut usize) -> Result SOROBAN_MAX_PENDING_WITHDRAWALS as usize { + return Err(RuntimeError::storage_error("withdraw queue too large")); + } + if next_withdraw_to_execute > next_pending_withdrawal_id { + return Err(RuntimeError::storage_error("withdraw queue invalid ids")); + } + if count == 0 && next_withdraw_to_execute != next_pending_withdrawal_id { + return Err(RuntimeError::storage_error( + "empty withdraw queue invalid ids", + )); + } let mut entries = Vec::with_capacity(count); + let mut previous_id = None; for _ in 0..count { let id = read_u64(bytes, cursor)?; + if id < next_withdraw_to_execute || id >= next_pending_withdrawal_id { + return Err(RuntimeError::storage_error( + "withdraw queue id out of range", + )); + } + if let Some(previous) = previous_id { + if id <= previous { + return Err(RuntimeError::storage_error("withdraw queue ids unsorted")); + } + } else if id != next_withdraw_to_execute { + return Err(RuntimeError::storage_error("withdraw queue head missing")); + } + previous_id = Some(id); let withdrawal = PendingWithdrawal::new( read_address(bytes, cursor)?, read_address(bytes, cursor)?, @@ -426,11 +503,18 @@ fn decode_withdraw_queue(bytes: &[u8], cursor: &mut usize) -> Result) { @@ -537,6 +621,7 @@ fn decode_op_state(bytes: &[u8], cursor: &mut usize) -> Result Vec { let mut out = Vec::new(); + push_storage_header(&mut out, STORAGE_KIND_STATE); push_u128(&mut out, state.total_assets); push_u128(&mut out, state.total_shares); push_u128(&mut out, state.idle_assets); @@ -550,6 +635,7 @@ pub(crate) fn encode_state_blob(state: &VaultState) -> Vec { } pub(crate) fn decode_state_blob(bytes: &[u8]) -> Result { + let bytes = storage_payload(bytes, STORAGE_KIND_STATE)?; let mut cursor = 0usize; let state = VaultState { total_assets: read_u128(bytes, &mut cursor)?, @@ -590,14 +676,14 @@ pub(crate) fn compose_policy_state( return Ok(None); } - let state = PolicyState::from_parts( - markets.unwrap_or_default(), - principals.unwrap_or_default(), - cap_groups.unwrap_or_default(), - leases.unwrap_or_default(), - supply_queue.unwrap_or_default(), - ) - .map_err(|_| RuntimeError::storage_error(""))?; + let (Some(markets), Some(principals), Some(cap_groups), Some(leases), Some(supply_queue)) = + (markets, principals, cap_groups, leases, supply_queue) + else { + return Err(RuntimeError::storage_error("partial policy state")); + }; + + let state = PolicyState::from_parts(markets, principals, cap_groups, leases, supply_queue) + .map_err(|_| RuntimeError::storage_error(""))?; Ok(Some(state)) } @@ -649,7 +735,11 @@ impl<'a> SorobanStorage<'a> { } /// Save a kernel-to-Soroban address mapping to persistent storage. - pub fn save_address(&self, kernel_addr: &Address, soroban_addr: &SdkAddress) { + pub fn save_address( + &self, + kernel_addr: &Address, + soroban_addr: &SdkAddress, + ) -> Result<(), RuntimeError> { let key = self.address_key(kernel_addr); self.env.storage().persistent().set(&key, soroban_addr); self.env.storage().persistent().extend_ttl( @@ -658,6 +748,33 @@ impl<'a> SorobanStorage<'a> { DEFAULT_TTL_EXTEND_TO, ); self.extend_default_ttl(); + Ok(()) + } + + pub(crate) fn extend_address_ttl(&self, kernel_addr: &Address, threshold: u32, extend_to: u32) { + let key = self.address_key(kernel_addr); + let p = self.env.storage().persistent(); + if p.has(&key) { + p.extend_ttl(&key, threshold, extend_to); + } + } + + fn extend_state_address_ttls(&self, state: &VaultState, threshold: u32, extend_to: u32) { + for (_, withdrawal) in state.withdraw_queue.iter() { + self.extend_address_ttl(&withdrawal.owner, threshold, extend_to); + self.extend_address_ttl(&withdrawal.receiver, threshold, extend_to); + } + match &state.op_state { + OpState::Withdrawing(withdrawing) => { + self.extend_address_ttl(&withdrawing.owner, threshold, extend_to); + self.extend_address_ttl(&withdrawing.receiver, threshold, extend_to); + } + OpState::Payout(payout) => { + self.extend_address_ttl(&payout.owner, threshold, extend_to); + self.extend_address_ttl(&payout.receiver, threshold, extend_to); + } + _ => {} + } } pub(crate) fn load_state_blob(&self) -> Option> { @@ -801,6 +918,11 @@ impl<'a> SorobanStorage<'a> { p.extend_ttl(key, threshold, extend_to); } } + if let Some(stored) = self.load_state_blob() { + if let Ok(state) = decode_state_blob(&stored) { + self.extend_state_address_ttls(&state, threshold, extend_to); + } + } } fn extend_default_ttl(&self) { @@ -818,7 +940,16 @@ impl Storage for SorobanStorage<'_> { } fn save_state(&mut self, state: &VaultState) -> Result<(), RuntimeError> { + if !state + .withdraw_queue + .check_invariants_with_max(SOROBAN_MAX_PENDING_WITHDRAWALS) + { + return Err(RuntimeError::storage_error( + "withdraw queue exceeds soroban cap", + )); + } let state_blob = encode_state_blob(state); + ensure_contract_data_entry_size(&state_blob)?; self.save_state_blob(&state_blob); self.extend_default_ttl(); Ok(()) @@ -864,11 +995,19 @@ impl Storage for SorobanStorage<'_> { } fn save_policy_state(&mut self, state: &PolicyState) -> Result<(), RuntimeError> { - self.save_policy_locks(&encode_policy_locks(state.leases())); - self.save_policy_supply_queue(&encode_supply_queue(state.supply_queue())); - self.save_policy_markets(&encode_markets(state.markets())); - self.save_policy_principals(&encode_principals(state.principals())); - self.save_policy_cap_groups(&encode_cap_groups(state.cap_groups())); + let locks = encode_policy_locks(state.leases()); + let supply_queue = encode_supply_queue(state.supply_queue()); + let markets = encode_markets(state.markets()); + let principals = encode_principals(state.principals()); + let cap_groups = encode_cap_groups(state.cap_groups()); + for bytes in [&locks, &supply_queue, &markets, &principals, &cap_groups] { + ensure_contract_data_entry_size(bytes)?; + } + self.save_policy_locks(&locks); + self.save_policy_supply_queue(&supply_queue); + self.save_policy_markets(&markets); + self.save_policy_principals(&principals); + self.save_policy_cap_groups(&cap_groups); self.extend_default_ttl(); Ok(()) } @@ -887,7 +1026,17 @@ impl Storage for SorobanStorage<'_> { restrictions: &Option, ) -> Result<(), RuntimeError> { if let Some(restrictions) = restrictions { - SorobanStorage::save_restrictions(self, &encode_restrictions(restrictions)); + let len = match restrictions { + Restrictions::Blacklist(addresses) | Restrictions::Whitelist(addresses) => { + addresses.len() + } + }; + if len > SOROBAN_MAX_RESTRICTION_ADDRESSES { + return Err(RuntimeError::storage_error("restrictions too large")); + } + let bytes = encode_restrictions(restrictions); + ensure_contract_data_entry_size(&bytes)?; + SorobanStorage::save_restrictions(self, &bytes); } else { SorobanStorage::clear_restrictions(self); } @@ -904,8 +1053,7 @@ impl Storage for SorobanStorage<'_> { kernel_addr: &Address, soroban_addr: &SdkAddress, ) -> Result<(), RuntimeError> { - SorobanStorage::save_address(self, kernel_addr, soroban_addr); - Ok(()) + SorobanStorage::save_address(self, kernel_addr, soroban_addr) } } diff --git a/contract/vault/soroban/src/tests.rs b/contract/vault/soroban/src/tests.rs index e15a8b8db..d13e7682b 100644 --- a/contract/vault/soroban/src/tests.rs +++ b/contract/vault/soroban/src/tests.rs @@ -1964,7 +1964,10 @@ mod storage_tests { use crate::contract::helpers::set_config_address; use crate::contract::SorobanVaultContract; use crate::error::RuntimeError; - use crate::storage::{SorobanStorage, SorobanStorageKey, Storage}; + use crate::storage::{ + SorobanStorage, SorobanStorageKey, Storage, SOROBAN_MAX_PENDING_WITHDRAWALS, + SOROBAN_MAX_RESTRICTION_ADDRESSES, + }; use crate::test_utils::{fuzz_api, MemoryStorage}; use alloc::string::{String as AllocString, ToString}; use rstest::{fixture, rstest}; @@ -2145,6 +2148,38 @@ mod storage_tests { }); } + #[rstest] + fn soroban_storage_extends_live_address_book_entries_from_state( + contract_env: (Env, soroban_sdk::Address), + ) { + let (env, contract_id) = contract_env; + env.as_contract(&contract_id, || { + let mut storage = SorobanStorage::new(&env); + let kernel_addr = KernelAddress([9u8; 32]); + let sdk_addr = SdkAddress::generate(&env); + + storage.save_address(&kernel_addr, &sdk_addr).unwrap(); + let state = VaultState { + withdraw_queue: WithdrawQueue::with_state( + alloc::vec![( + 0, + PendingWithdrawal::new(kernel_addr, kernel_addr, 1, 1, TimestampNs(0),), + )], + 0, + 1, + ), + ..Default::default() + }; + storage.save_state(&state).unwrap(); + + assert_eq!( + Storage::load_address(&storage, &kernel_addr).unwrap(), + Some(sdk_addr) + ); + storage.extend_ttl(1, 100); + }); + } + #[rstest] fn test_soroban_storage_pause_state(contract_env: (Env, soroban_sdk::Address)) { let (env, contract_id) = contract_env; @@ -2291,6 +2326,114 @@ mod storage_tests { assert_eq!(decoded, state); } + fn state_with_pending_withdrawals(count: u32) -> VaultState { + let pending = (0..u64::from(count)) + .map(|id| { + ( + id, + PendingWithdrawal::new( + KernelAddress([1u8; 32]), + KernelAddress([2u8; 32]), + 1, + 1, + TimestampNs(id), + ), + ) + }) + .collect::>(); + let mut state = VaultState::default(); + state.withdraw_queue = WithdrawQueue::with_state(pending, 0, u64::from(count)); + state + } + + #[test] + fn storage_codec_state_blob_is_versioned_and_legacy_decodes() { + let state = state_with_pending_withdrawals(1); + let encoded = fuzz_api::encode_state_blob_bytes(&state); + assert_eq!(&encoded[..3], b"TVS"); + + let decoded = fuzz_api::decode_state_blob_bytes(&encoded).expect("versioned state"); + assert_eq!(decoded, state); + + let legacy = encoded[5..].to_vec(); + let decoded_legacy = + fuzz_api::decode_state_blob_bytes(&legacy).expect("legacy state still decodes"); + assert_eq!(decoded_legacy, state); + } + + #[test] + fn migrate_validates_and_rewrites_legacy_state_blob() { + let env = Env::default(); + env.mock_all_auths_allowing_non_root_auth(); + let contract_id = env.register(SorobanVaultContract, ()); + let curator = SdkAddress::generate(&env); + let governance = SdkAddress::generate(&env); + let asset = SdkAddress::generate(&env); + let share = SdkAddress::generate(&env); + + env.as_contract(&contract_id, || { + SorobanVaultContract::initialize( + env.clone(), + curator, + governance.clone(), + asset, + share, + 0, + 0, + ) + .unwrap(); + + let storage = SorobanStorage::new(&env); + let legacy = fuzz_api::encode_state_blob_bytes(&VaultState::default())[5..].to_vec(); + storage.save_state_blob(&legacy); + env.storage() + .instance() + .set(&soroban_sdk::symbol_short!("migrate"), &true); + + SorobanVaultContract::migrate(env.clone(), governance).unwrap(); + + let stored = storage.load_state_blob().expect("rewritten state blob"); + assert_eq!(&stored[..3], b"TVS"); + assert_eq!( + env.storage() + .instance() + .get::<_, bool>(&soroban_sdk::symbol_short!("migrate")), + None + ); + }); + } + + #[test] + fn storage_codec_rejects_malformed_withdraw_queue_ids() { + let state = state_with_pending_withdrawals(2); + let mut encoded = fuzz_api::encode_state_blob_bytes(&state); + let second_id_offset = 5 + 89 + 8 + 8 + 4 + 112; + encoded[second_id_offset..second_id_offset + 8].copy_from_slice(&0u64.to_le_bytes()); + + assert!(fuzz_api::decode_state_blob_bytes(&encoded).is_err()); + } + + #[rstest] + fn soroban_storage_enforces_safe_withdraw_queue_cap(contract_env: (Env, soroban_sdk::Address)) { + let (env, contract_id) = contract_env; + env.as_contract(&contract_id, || { + let mut storage = SorobanStorage::new(&env); + let safe = state_with_pending_withdrawals(SOROBAN_MAX_PENDING_WITHDRAWALS); + assert!(fuzz_api::encode_state_blob_bytes(&safe).len() < 64 * 1024); + storage.save_state(&safe).expect("safe queue cap persists"); + + let too_large = state_with_pending_withdrawals(SOROBAN_MAX_PENDING_WITHDRAWALS + 1); + assert!(fuzz_api::encode_state_blob_bytes(&too_large).len() < 64 * 1024); + assert_eq!( + storage.save_state(&too_large).unwrap_err(), + RuntimeError::StorageError + ); + + let kernel_max = state_with_pending_withdrawals(1024); + assert!(fuzz_api::encode_state_blob_bytes(&kernel_max).len() > 64 * 1024); + }); + } + #[test] fn storage_codec_roundtrip_restrictions() { let restrictions = Restrictions::blacklist(alloc::vec![ @@ -2307,6 +2450,29 @@ mod storage_tests { assert!(fuzz_api::decode_restrictions_bytes(&trailing).is_err()); } + #[rstest] + fn soroban_storage_rejects_oversized_restrictions_before_save( + contract_env: (Env, soroban_sdk::Address), + ) { + let (env, contract_id) = contract_env; + env.as_contract(&contract_id, || { + let mut storage = SorobanStorage::new(&env); + let addresses = (0..=SOROBAN_MAX_RESTRICTION_ADDRESSES) + .map(|i| { + let mut raw = [0u8; 32]; + raw[..8].copy_from_slice(&(i as u64).to_le_bytes()); + KernelAddress(raw) + }) + .collect::>(); + let restrictions = Some(Restrictions::blacklist(addresses)); + + assert_eq!( + Storage::save_restrictions(&mut storage, &restrictions).unwrap_err(), + RuntimeError::StorageError + ); + }); + } + #[rstest] #[case::empty(alloc::vec![])] #[case::tag_only(alloc::vec![0])] @@ -2404,6 +2570,22 @@ mod storage_tests { } } + #[rstest] + fn soroban_storage_rejects_partial_policy_state(contract_env: (Env, soroban_sdk::Address)) { + let (env, contract_id) = contract_env; + env.as_contract(&contract_id, || { + let storage = SorobanStorage::new(&env); + let mut markets = OrderedMap::new(); + let _ = markets.insert(1, MarketConfig::new(true, 100, None)); + storage.save_policy_markets(&fuzz_api::encode_markets_bytes(&markets)); + + assert_eq!( + Storage::load_policy_state(&storage).unwrap_err(), + RuntimeError::StorageError + ); + }); + } + #[test] fn storage_codec_roundtrip_markets_and_invalid_tags_fail_cleanly() { use alloc::string::String; From 16fea2ce03ff63a08b72e68410669f7f48b17db2 Mon Sep 17 00:00:00 2001 From: carrion256 Date: Tue, 5 May 2026 18:08:01 +0200 Subject: [PATCH 2/4] fix: require versioned storage envelopes (#FIND-016 Nexus 3966d571-d2e2-43ba-aff1-def6813725e5) Trace: fcab9d3c034b3fc312c3f10cbe9f9901868ff266 --- contract/vault/soroban/README.md | 2 +- contract/vault/soroban/src/storage/mod.rs | 130 +++++++++++++++++++--- contract/vault/soroban/src/tests.rs | 19 ++-- 3 files changed, 127 insertions(+), 24 deletions(-) diff --git a/contract/vault/soroban/README.md b/contract/vault/soroban/README.md index ab3b0caa8..e154b6721 100644 --- a/contract/vault/soroban/README.md +++ b/contract/vault/soroban/README.md @@ -197,7 +197,7 @@ Useful commands: - Vault runtime state is persisted as a single versioned `StateBlob`, so serialized `VaultState` size is the practical storage-pressure point. - The shared kernel still has an absolute `MAX_PENDING = 1024`, but Soroban uses the chain-specific `SOROBAN_MAX_PENDING_WITHDRAWALS = 512` runtime cap to stay below the 64 KiB contract-data-entry limit with room for schema growth. - In-flight operation plans (`Allocating.plan`, `Refreshing.plan`) are expected to remain small under allocator policy, so the 512 Soroban pending-withdrawal cap is the dominant operational bound in practice. -- Persistent storage blobs carry a compact `TVS` version header on new writes. Decoders still accept the pre-header legacy layout so governed `migrate()` can validate and rewrite old blobs in place. +- Persistent storage blobs carry a compact `TVS` version header. Decoders reject pre-header bytes and unsupported versions; schema upgrades should add explicit per-version decode/migration dispatch before any layout change. ## Practical Risk Model diff --git a/contract/vault/soroban/src/storage/mod.rs b/contract/vault/soroban/src/storage/mod.rs index c32f0aa78..51c2f6aba 100644 --- a/contract/vault/soroban/src/storage/mod.rs +++ b/contract/vault/soroban/src/storage/mod.rs @@ -42,6 +42,31 @@ const STORAGE_KIND_PRINCIPALS: u8 = 5; const STORAGE_KIND_CAP_GROUPS: u8 = 6; const STORAGE_KIND_POLICY_LOCKS: u8 = 7; +#[derive(Clone, Copy)] +enum StorageKind { + State, + Restrictions, + SupplyQueue, + Markets, + Principals, + CapGroups, + PolicyLocks, +} + +impl StorageKind { + const fn tag(self) -> u8 { + match self { + Self::State => STORAGE_KIND_STATE, + Self::Restrictions => STORAGE_KIND_RESTRICTIONS, + Self::SupplyQueue => STORAGE_KIND_SUPPLY_QUEUE, + Self::Markets => STORAGE_KIND_MARKETS, + Self::Principals => STORAGE_KIND_PRINCIPALS, + Self::CapGroups => STORAGE_KIND_CAP_GROUPS, + Self::PolicyLocks => STORAGE_KIND_POLICY_LOCKS, + } + } +} + /// Internal persistent storage keys. Using Symbol constants instead of a /// `#[contracttype]` enum to avoid contractspec bloat and enum conversion codegen. #[allow(non_upper_case_globals)] @@ -66,14 +91,40 @@ fn push_storage_header(out: &mut Vec, kind: u8) { push_u8(out, STORAGE_VERSION_CURRENT); } -fn storage_payload(bytes: &[u8], kind: u8) -> Result<&[u8], RuntimeError> { - if bytes.len() >= 5 && bytes[..3] == STORAGE_MAGIC && bytes[3] == kind { - return match bytes[4] { - STORAGE_VERSION_CURRENT => Ok(&bytes[5..]), - _ => Err(RuntimeError::storage_error("unsupported storage version")), - }; +fn storage_payload(bytes: &[u8], kind: StorageKind) -> Result<(u8, &[u8]), RuntimeError> { + if bytes.len() < 5 || bytes[..3] != STORAGE_MAGIC || bytes[3] != kind.tag() { + return Err(RuntimeError::storage_error("invalid storage envelope")); + } + Ok((bytes[4], &bytes[5..])) +} + +struct StorageVersionDecoder { + version: u8, + decode: fn(&[u8]) -> Result, +} + +impl StorageVersionDecoder { + const fn current(decode: fn(&[u8]) -> Result) -> Self { + Self { + version: STORAGE_VERSION_CURRENT, + decode, + } } - Ok(bytes) +} + +fn decode_storage_payload( + bytes: &[u8], + kind: StorageKind, + decoders: &[StorageVersionDecoder], +) -> Result { + let (version, payload) = storage_payload(bytes, kind)?; + for decoder in decoders { + if decoder.version == version { + return (decoder.decode)(payload); + } + } + + Err(RuntimeError::storage_error("unsupported storage version")) } fn ensure_contract_data_entry_size(bytes: &[u8]) -> Result<(), RuntimeError> { @@ -198,7 +249,14 @@ pub(crate) fn encode_restrictions(mode: &Restrictions) -> Vec { } pub(crate) fn decode_restrictions(bytes: &[u8]) -> Result { - let bytes = storage_payload(bytes, STORAGE_KIND_RESTRICTIONS)?; + decode_storage_payload( + bytes, + StorageKind::Restrictions, + &[StorageVersionDecoder::current(decode_restrictions_v1)], + ) +} + +fn decode_restrictions_v1(bytes: &[u8]) -> Result { let mut cursor = 0usize; let tag = read_u8(bytes, &mut cursor)?; let len = read_u32(bytes, &mut cursor)? as usize; @@ -234,7 +292,14 @@ pub(crate) fn encode_supply_queue(queue: &SupplyQueue) -> Vec { } pub(crate) fn decode_supply_queue(bytes: &[u8]) -> Result { - let bytes = storage_payload(bytes, STORAGE_KIND_SUPPLY_QUEUE)?; + decode_storage_payload( + bytes, + StorageKind::SupplyQueue, + &[StorageVersionDecoder::current(decode_supply_queue_v1)], + ) +} + +fn decode_supply_queue_v1(bytes: &[u8]) -> Result { let mut cursor = 0usize; let max_length = read_u32(bytes, &mut cursor)?; let count = read_u32(bytes, &mut cursor)? as usize; @@ -282,7 +347,16 @@ pub(crate) fn encode_cap_groups(cap_groups: &OrderedMap Result, RuntimeError> { - let bytes = storage_payload(bytes, STORAGE_KIND_CAP_GROUPS)?; + decode_storage_payload( + bytes, + StorageKind::CapGroups, + &[StorageVersionDecoder::current(decode_cap_groups_v1)], + ) +} + +fn decode_cap_groups_v1( + bytes: &[u8], +) -> Result, RuntimeError> { let mut cursor = 0usize; let count = read_u32(bytes, &mut cursor)? as usize; let mut cap_groups = OrderedMap::new(); @@ -338,7 +412,14 @@ pub(crate) fn encode_markets(markets: &OrderedMap) -> Ve pub(crate) fn decode_markets( bytes: &[u8], ) -> Result, RuntimeError> { - let bytes = storage_payload(bytes, STORAGE_KIND_MARKETS)?; + decode_storage_payload( + bytes, + StorageKind::Markets, + &[StorageVersionDecoder::current(decode_markets_v1)], + ) +} + +fn decode_markets_v1(bytes: &[u8]) -> Result, RuntimeError> { let mut cursor = 0usize; let count = read_u32(bytes, &mut cursor)? as usize; let mut markets = OrderedMap::new(); @@ -373,7 +454,14 @@ pub(crate) fn encode_principals(principals: &OrderedMap) -> Vec< } pub(crate) fn decode_principals(bytes: &[u8]) -> Result, RuntimeError> { - let bytes = storage_payload(bytes, STORAGE_KIND_PRINCIPALS)?; + decode_storage_payload( + bytes, + StorageKind::Principals, + &[StorageVersionDecoder::current(decode_principals_v1)], + ) +} + +fn decode_principals_v1(bytes: &[u8]) -> Result, RuntimeError> { let mut cursor = 0usize; let count = read_u32(bytes, &mut cursor)? as usize; let mut principals = OrderedMap::new(); @@ -409,7 +497,14 @@ pub(crate) fn encode_policy_locks(leases: &MarketLeaseRegistry) -> Vec { } pub(crate) fn decode_policy_locks(bytes: &[u8]) -> Result { - let bytes = storage_payload(bytes, STORAGE_KIND_POLICY_LOCKS)?; + decode_storage_payload( + bytes, + StorageKind::PolicyLocks, + &[StorageVersionDecoder::current(decode_policy_locks_v1)], + ) +} + +fn decode_policy_locks_v1(bytes: &[u8]) -> Result { let mut cursor = 0usize; let count = read_u32(bytes, &mut cursor)? as usize; let next_fencing_token = read_u64(bytes, &mut cursor)?; @@ -635,7 +730,14 @@ pub(crate) fn encode_state_blob(state: &VaultState) -> Vec { } pub(crate) fn decode_state_blob(bytes: &[u8]) -> Result { - let bytes = storage_payload(bytes, STORAGE_KIND_STATE)?; + decode_storage_payload( + bytes, + StorageKind::State, + &[StorageVersionDecoder::current(decode_state_blob_v1)], + ) +} + +fn decode_state_blob_v1(bytes: &[u8]) -> Result { let mut cursor = 0usize; let state = VaultState { total_assets: read_u128(bytes, &mut cursor)?, diff --git a/contract/vault/soroban/src/tests.rs b/contract/vault/soroban/src/tests.rs index d13e7682b..889dd592e 100644 --- a/contract/vault/soroban/src/tests.rs +++ b/contract/vault/soroban/src/tests.rs @@ -2347,7 +2347,7 @@ mod storage_tests { } #[test] - fn storage_codec_state_blob_is_versioned_and_legacy_decodes() { + fn storage_codec_state_blob_requires_versioned_envelope() { let state = state_with_pending_withdrawals(1); let encoded = fuzz_api::encode_state_blob_bytes(&state); assert_eq!(&encoded[..3], b"TVS"); @@ -2355,14 +2355,15 @@ mod storage_tests { let decoded = fuzz_api::decode_state_blob_bytes(&encoded).expect("versioned state"); assert_eq!(decoded, state); - let legacy = encoded[5..].to_vec(); - let decoded_legacy = - fuzz_api::decode_state_blob_bytes(&legacy).expect("legacy state still decodes"); - assert_eq!(decoded_legacy, state); + assert!(fuzz_api::decode_state_blob_bytes(&encoded[5..]).is_err()); + + let mut unsupported_version = encoded; + unsupported_version[4] = 2; + assert!(fuzz_api::decode_state_blob_bytes(&unsupported_version).is_err()); } #[test] - fn migrate_validates_and_rewrites_legacy_state_blob() { + fn migrate_validates_current_version_state_blob() { let env = Env::default(); env.mock_all_auths_allowing_non_root_auth(); let contract_id = env.register(SorobanVaultContract, ()); @@ -2384,8 +2385,8 @@ mod storage_tests { .unwrap(); let storage = SorobanStorage::new(&env); - let legacy = fuzz_api::encode_state_blob_bytes(&VaultState::default())[5..].to_vec(); - storage.save_state_blob(&legacy); + let versioned = fuzz_api::encode_state_blob_bytes(&VaultState::default()); + storage.save_state_blob(&versioned); env.storage() .instance() .set(&soroban_sdk::symbol_short!("migrate"), &true); @@ -2393,7 +2394,7 @@ mod storage_tests { SorobanVaultContract::migrate(env.clone(), governance).unwrap(); let stored = storage.load_state_blob().expect("rewritten state blob"); - assert_eq!(&stored[..3], b"TVS"); + assert_eq!(stored, versioned); assert_eq!( env.storage() .instance() From 0daab1a05e588d40b4131cb6e6551d7596742792 Mon Sep 17 00:00:00 2001 From: carrion256 Date: Tue, 5 May 2026 18:36:48 +0200 Subject: [PATCH 3/4] fix: page Soroban withdrawal storage (#FIND-017 #FIND-037 Nexus 49a0ad1a-499b-4ace-b7b4-ceaca30a5cf3 Nexus a21aac55-d33b-4841-8427-a0466ac23fe7) Trace: fee307b82177c0beeaccf751c5f785c6c9cffec8 --- contract/vault/soroban/README.md | 7 +- contract/vault/soroban/src/storage/mod.rs | 611 +++++++++++++++++++--- contract/vault/soroban/src/tests.rs | 65 ++- 3 files changed, 600 insertions(+), 83 deletions(-) diff --git a/contract/vault/soroban/README.md b/contract/vault/soroban/README.md index e154b6721..75c06724a 100644 --- a/contract/vault/soroban/README.md +++ b/contract/vault/soroban/README.md @@ -194,9 +194,10 @@ Useful commands: ## State Size and Operational Limits - Soroban enforces per-entry and per-transaction resource limits. Current network values are documented by Stellar: https://developers.stellar.org/docs/networks/resource-limits-fees -- Vault runtime state is persisted as a single versioned `StateBlob`, so serialized `VaultState` size is the practical storage-pressure point. -- The shared kernel still has an absolute `MAX_PENDING = 1024`, but Soroban uses the chain-specific `SOROBAN_MAX_PENDING_WITHDRAWALS = 512` runtime cap to stay below the 64 KiB contract-data-entry limit with room for schema growth. -- In-flight operation plans (`Allocating.plan`, `Refreshing.plan`) are expected to remain small under allocator policy, so the 512 Soroban pending-withdrawal cap is the dominant operational bound in practice. +- Vault runtime state is persisted as a compact versioned `StateBlob` header plus domain-paged withdrawal queue entries. Each `wqpage` stores up to 128 pending withdrawals, so the queue can use the kernel `MAX_PENDING = 1024` cap without coupling the whole queue to one 64 KiB storage entry. +- Restrictions and policy blobs use the generic blob-paging transport. Small payloads are stored inline; larger payloads are split into bounded 32 KiB pages. +- One contract invocation is still bounded by Soroban transaction resource limits. Very large sanctions-list style updates should use a batched governance/update flow instead of one giant replacement payload. +- In-flight operation plans (`Allocating.plan`, `Refreshing.plan`) are expected to remain small under allocator policy; if that assumption changes, the paged blob transport protects storage entry size but not per-transaction CPU/write-byte budgets. - Persistent storage blobs carry a compact `TVS` version header. Decoders reject pre-header bytes and unsupported versions; schema upgrades should add explicit per-version decode/migration dispatch before any layout change. ## Practical Risk Model diff --git a/contract/vault/soroban/src/storage/mod.rs b/contract/vault/soroban/src/storage/mod.rs index 51c2f6aba..d87a2a85e 100644 --- a/contract/vault/soroban/src/storage/mod.rs +++ b/contract/vault/soroban/src/storage/mod.rs @@ -28,12 +28,19 @@ pub(crate) const DEFAULT_TTL_THRESHOLD: u32 = 518_400; /// For a vault contract holding real assets, maximum TTL prevents state /// loss during extended pauses or periods of inactivity. pub(crate) const DEFAULT_TTL_EXTEND_TO: u32 = 3_110_400; -pub(crate) const SOROBAN_MAX_PENDING_WITHDRAWALS: u32 = 512; -pub(crate) const SOROBAN_MAX_RESTRICTION_ADDRESSES: usize = 1024; +pub(crate) const SOROBAN_MAX_PENDING_WITHDRAWALS: u32 = templar_vault_kernel::MAX_PENDING as u32; +pub(crate) const SOROBAN_MAX_RESTRICTION_ADDRESSES: usize = 3_072; const MAX_CONTRACT_DATA_ENTRY_SIZE_BYTES: usize = 64 * 1024; +const BLOB_PAGE_BYTES: usize = 32 * 1024; +const BLOB_MAGIC: [u8; 3] = *b"TVP"; +const BLOB_VERSION_CURRENT: u8 = 1; +const BLOB_INLINE: u8 = 0; +const BLOB_PAGED: u8 = 1; const STORAGE_MAGIC: [u8; 3] = *b"TVS"; const STORAGE_VERSION_CURRENT: u8 = 1; +const STORAGE_VERSION_STATE_PAGED: u8 = 2; +const WITHDRAW_QUEUE_PAGE_SIZE: u64 = 128; const STORAGE_KIND_STATE: u8 = 1; const STORAGE_KIND_RESTRICTIONS: u8 = 2; const STORAGE_KIND_SUPPLY_QUEUE: u8 = 3; @@ -85,10 +92,14 @@ impl SorobanStorageKey { pub const PausedState: Symbol = symbol_short!("paused_s"); } -fn push_storage_header(out: &mut Vec, kind: u8) { +fn push_storage_header_version(out: &mut Vec, kind: u8, version: u8) { out.extend_from_slice(&STORAGE_MAGIC); push_u8(out, kind); - push_u8(out, STORAGE_VERSION_CURRENT); + push_u8(out, version); +} + +fn push_storage_header(out: &mut Vec, kind: u8) { + push_storage_header_version(out, kind, STORAGE_VERSION_CURRENT); } fn storage_payload(bytes: &[u8], kind: StorageKind) -> Result<(u8, &[u8]), RuntimeError> { @@ -135,6 +146,74 @@ fn ensure_contract_data_entry_size(bytes: &[u8]) -> Result<(), RuntimeError> { } } +fn push_blob_header(out: &mut Vec, mode: u8) { + out.extend_from_slice(&BLOB_MAGIC); + push_u8(out, BLOB_VERSION_CURRENT); + push_u8(out, mode); +} + +fn encode_blob_inline(bytes: &[u8]) -> Vec { + let mut out = Vec::with_capacity(9 + bytes.len()); + push_blob_header(&mut out, BLOB_INLINE); + push_u32(&mut out, bytes.len() as u32); + out.extend_from_slice(bytes); + out +} + +fn encode_blob_manifest(bytes_len: usize, page_count: u32) -> Vec { + let mut out = Vec::with_capacity(13); + push_blob_header(&mut out, BLOB_PAGED); + push_u32(&mut out, bytes_len as u32); + push_u32(&mut out, page_count); + out +} + +#[derive(Clone, Copy, PartialEq, Eq)] +enum BlobManifest { + Inline, + Paged { len: usize, page_count: u32 }, +} + +fn decode_blob_manifest(bytes: &[u8]) -> Result, RuntimeError> { + if bytes.len() < 5 || bytes[..3] != BLOB_MAGIC { + return Ok(None); + } + + let version = bytes[3]; + if version != BLOB_VERSION_CURRENT { + return Err(RuntimeError::storage_error( + "unsupported blob storage version", + )); + } + + let mut cursor = 4usize; + match read_u8(bytes, &mut cursor)? { + BLOB_INLINE => { + let len = read_u32(bytes, &mut cursor)? as usize; + if bytes.len().saturating_sub(cursor) != len { + return Err(RuntimeError::storage_error("invalid inline blob")); + } + Ok(Some(BlobManifest::Inline)) + } + BLOB_PAGED => { + let len = read_u32(bytes, &mut cursor)? as usize; + let page_count = read_u32(bytes, &mut cursor)?; + finish_decode(bytes, cursor)?; + if page_count == 0 || len == 0 { + return Err(RuntimeError::storage_error("invalid paged blob")); + } + let capacity = (page_count as usize) + .checked_mul(BLOB_PAGE_BYTES) + .ok_or_else(|| RuntimeError::storage_error("paged blob too large"))?; + if len > capacity || len <= capacity.saturating_sub(BLOB_PAGE_BYTES) { + return Err(RuntimeError::storage_error("invalid paged blob length")); + } + Ok(Some(BlobManifest::Paged { len, page_count })) + } + _ => Err(RuntimeError::storage_error("invalid blob storage mode")), + } +} + fn push_u8(out: &mut Vec, value: u8) { out.push(value); } @@ -542,6 +621,7 @@ fn decode_policy_locks_v1(bytes: &[u8]) -> Result) { push_u64(out, queue.next_withdraw_to_execute); push_u64(out, queue.next_pending_withdrawal_id); @@ -557,6 +637,7 @@ fn encode_withdraw_queue(queue: &WithdrawQueue, out: &mut Vec) { } } +#[allow(dead_code, reason = "kept for storage codec regression and fuzz tests")] fn decode_withdraw_queue(bytes: &[u8], cursor: &mut usize) -> Result { let next_withdraw_to_execute = read_u64(bytes, cursor)?; let next_pending_withdrawal_id = read_u64(bytes, cursor)?; @@ -612,6 +693,129 @@ fn decode_withdraw_queue(bytes: &[u8], cursor: &mut usize) -> Result u64 { + withdrawal_id / WITHDRAW_QUEUE_PAGE_SIZE +} + +fn queue_page_range(queue: &WithdrawQueue) -> Option<(u64, u64)> { + if queue.is_empty() { + return None; + } + let tail_id = queue.next_pending_withdrawal_id.checked_sub(1)?; + Some(( + queue_page_id(queue.next_withdraw_to_execute), + queue_page_id(tail_id), + )) +} + +fn queue_header_page_range(header: WithdrawQueueHeader) -> Option<(u64, u64)> { + if header.next_withdraw_to_execute >= header.next_pending_withdrawal_id { + return None; + } + let tail_id = header.next_pending_withdrawal_id.checked_sub(1)?; + Some(( + queue_page_id(header.next_withdraw_to_execute), + queue_page_id(tail_id), + )) +} + +fn encode_withdraw_queue_header(queue: &WithdrawQueue, out: &mut Vec) { + push_u64(out, queue.next_withdraw_to_execute); + push_u64(out, queue.next_pending_withdrawal_id); + push_u32(out, queue.len() as u32); +} + +fn decode_withdraw_queue_header( + bytes: &[u8], + cursor: &mut usize, +) -> Result { + let header = WithdrawQueueHeader { + next_withdraw_to_execute: read_u64(bytes, cursor)?, + next_pending_withdrawal_id: read_u64(bytes, cursor)?, + pending_count: read_u32(bytes, cursor)?, + }; + if header.next_withdraw_to_execute > header.next_pending_withdrawal_id { + return Err(RuntimeError::storage_error("withdraw queue invalid ids")); + } + if header.pending_count > SOROBAN_MAX_PENDING_WITHDRAWALS { + return Err(RuntimeError::storage_error( + "withdraw queue exceeds soroban cap", + )); + } + if header.pending_count == 0 + && header.next_withdraw_to_execute != header.next_pending_withdrawal_id + { + return Err(RuntimeError::storage_error( + "empty withdraw queue invalid ids", + )); + } + if header.pending_count > 0 + && header.next_withdraw_to_execute >= header.next_pending_withdrawal_id + { + return Err(RuntimeError::storage_error( + "non-empty withdraw queue invalid ids", + )); + } + Ok(header) +} + +fn encode_withdraw_queue_page<'a>( + entries: impl IntoIterator, +) -> Vec { + let entries: Vec<_> = entries.into_iter().collect(); + let mut out = Vec::new(); + push_u32(&mut out, entries.len() as u32); + for (id, withdrawal) in entries { + push_u64(&mut out, id); + push_address(&mut out, &withdrawal.owner); + push_address(&mut out, &withdrawal.receiver); + push_u128(&mut out, withdrawal.escrow_shares); + push_u128(&mut out, withdrawal.expected_assets); + push_u64(&mut out, withdrawal.requested_at_ns.as_u64()); + } + out +} + +fn decode_withdraw_queue_page(bytes: &[u8]) -> Result, RuntimeError> { + let mut cursor = 0usize; + let count = read_u32(bytes, &mut cursor)? as usize; + if count > WITHDRAW_QUEUE_PAGE_SIZE as usize { + return Err(RuntimeError::storage_error("withdraw queue page too large")); + } + let mut entries = Vec::with_capacity(count); + for _ in 0..count { + let id = read_u64(bytes, &mut cursor)?; + let withdrawal = PendingWithdrawal::new( + read_address(bytes, &mut cursor)?, + read_address(bytes, &mut cursor)?, + read_u128(bytes, &mut cursor)?, + read_u128(bytes, &mut cursor)?, + templar_vault_kernel::TimestampNs(read_u64(bytes, &mut cursor)?), + ); + entries.push((id, withdrawal)); + } + finish_decode(bytes, cursor)?; + Ok(entries) +} + fn encode_op_state(op_state: &OpState, out: &mut Vec) { match op_state { OpState::Idle => push_u8(out, 0), @@ -714,6 +918,7 @@ fn decode_op_state(bytes: &[u8], cursor: &mut usize) -> Result Vec { let mut out = Vec::new(); push_storage_header(&mut out, STORAGE_KIND_STATE); @@ -729,6 +934,22 @@ pub(crate) fn encode_state_blob(state: &VaultState) -> Vec { out } +fn encode_state_header_blob(state: &VaultState) -> Vec { + let mut out = Vec::new(); + push_storage_header_version(&mut out, STORAGE_KIND_STATE, STORAGE_VERSION_STATE_PAGED); + push_u128(&mut out, state.total_assets); + push_u128(&mut out, state.total_shares); + push_u128(&mut out, state.idle_assets); + push_u128(&mut out, state.external_assets); + push_u128(&mut out, state.fee_anchor.total_assets); + push_u64(&mut out, state.fee_anchor.timestamp_ns.as_u64()); + encode_op_state(&state.op_state, &mut out); + encode_withdraw_queue_header(&state.withdraw_queue, &mut out); + push_u64(&mut out, state.next_op_id); + out +} + +#[allow(dead_code, reason = "kept for storage codec regression and fuzz tests")] pub(crate) fn decode_state_blob(bytes: &[u8]) -> Result { decode_storage_payload( bytes, @@ -737,6 +958,7 @@ pub(crate) fn decode_state_blob(bytes: &[u8]) -> Result Result { let mut cursor = 0usize; let state = VaultState { @@ -762,6 +984,61 @@ fn decode_state_blob_v1(bytes: &[u8]) -> Result { Ok(state) } +fn decode_state_header_blob(bytes: &[u8]) -> Result { + let (version, payload) = storage_payload(bytes, StorageKind::State)?; + if version != STORAGE_VERSION_STATE_PAGED { + return Err(RuntimeError::storage_error( + "unsupported state storage version", + )); + } + decode_state_header_blob_v2(payload) +} + +fn decode_state_header_blob_v2(bytes: &[u8]) -> Result { + let mut cursor = 0usize; + let header = StoredStateHeader { + total_assets: read_u128(bytes, &mut cursor)?, + total_shares: read_u128(bytes, &mut cursor)?, + idle_assets: read_u128(bytes, &mut cursor)?, + external_assets: read_u128(bytes, &mut cursor)?, + fee_anchor: FeeAccrualAnchor::new( + read_u128(bytes, &mut cursor)?, + templar_vault_kernel::TimestampNs(read_u64(bytes, &mut cursor)?), + ), + op_state: decode_op_state(bytes, &mut cursor)?, + withdraw_queue: decode_withdraw_queue_header(bytes, &mut cursor)?, + next_op_id: read_u64(bytes, &mut cursor)?, + }; + finish_decode(bytes, cursor)?; + Ok(header) +} + +fn compose_state_from_header( + header: StoredStateHeader, + withdraw_queue: WithdrawQueue, +) -> Result { + if withdraw_queue.next_withdraw_to_execute != header.withdraw_queue.next_withdraw_to_execute + || withdraw_queue.next_pending_withdrawal_id + != header.withdraw_queue.next_pending_withdrawal_id + || withdraw_queue.len() as u32 != header.withdraw_queue.pending_count + || !withdraw_queue.check_invariants_with_max(SOROBAN_MAX_PENDING_WITHDRAWALS) + { + return Err(RuntimeError::storage_error( + "withdraw queue pages do not match state header", + )); + } + Ok(VaultState { + total_assets: header.total_assets, + total_shares: header.total_shares, + idle_assets: header.idle_assets, + external_assets: header.external_assets, + fee_anchor: header.fee_anchor, + op_state: header.op_state, + withdraw_queue, + next_op_id: header.next_op_id, + }) +} + pub(crate) fn compose_policy_state( markets: Option>, principals: Option>, @@ -807,6 +1084,8 @@ impl<'a> SorobanStorage<'a> { } const SK_ADDRBOOK: Symbol = symbol_short!("addrbook"); + const SK_BLOBPAGE: Symbol = symbol_short!("blobpage"); + const SK_WQPAGE: Symbol = symbol_short!("wqpage"); fn address_key(&self, kernel_addr: &Address) -> (Symbol, BytesN<32>) { ( @@ -815,19 +1094,224 @@ impl<'a> SorobanStorage<'a> { ) } - fn load_blob(&self, key: &Symbol) -> Option> { - self.env - .storage() - .persistent() - .get::<_, Bytes>(key) - .map(|bytes| bytes.to_alloc_vec()) + fn blob_page_key(&self, key: &Symbol, page: u32) -> (Symbol, Symbol, u32) { + (Self::SK_BLOBPAGE, key.clone(), page) + } + + fn withdraw_queue_page_key(&self, page: u64) -> (Symbol, u64) { + (Self::SK_WQPAGE, page) + } + + fn stored_blob_manifest(&self, key: &Symbol) -> Result, RuntimeError> { + let Some(bytes) = self.env.storage().persistent().get::<_, Bytes>(key) else { + return Ok(None); + }; + decode_blob_manifest(&bytes.to_alloc_vec()) + } + + fn load_blob(&self, key: &Symbol) -> Result>, RuntimeError> { + let Some(stored) = self.env.storage().persistent().get::<_, Bytes>(key) else { + return Ok(None); + }; + let stored = stored.to_alloc_vec(); + match decode_blob_manifest(&stored)? { + Some(BlobManifest::Inline) => { + let mut cursor = 5usize; + let len = read_u32(&stored, &mut cursor)? as usize; + Ok(Some(stored[cursor..cursor + len].to_vec())) + } + Some(BlobManifest::Paged { len, page_count }) => { + let mut out = Vec::with_capacity(len); + let p = self.env.storage().persistent(); + for page in 0..page_count { + let page_key = self.blob_page_key(key, page); + let page_bytes = p + .get::<_, Bytes>(&page_key) + .ok_or_else(|| RuntimeError::storage_error("missing blob page"))? + .to_alloc_vec(); + if page_bytes.len() > BLOB_PAGE_BYTES { + return Err(RuntimeError::storage_error("blob page too large")); + } + out.extend_from_slice(&page_bytes); + } + if out.len() != len { + return Err(RuntimeError::storage_error("invalid paged blob length")); + } + Ok(Some(out)) + } + None => Err(RuntimeError::storage_error("invalid blob storage envelope")), + } } - fn save_blob(&self, key: &Symbol, bytes: &[u8]) { + fn save_blob(&self, key: &Symbol, bytes: &[u8]) -> Result<(), RuntimeError> { + let previous_page_count = match self.stored_blob_manifest(key)? { + Some(BlobManifest::Paged { page_count, .. }) => page_count, + _ => 0, + }; + + let p = self.env.storage().persistent(); + if bytes.len() <= BLOB_PAGE_BYTES { + let stored = encode_blob_inline(bytes); + ensure_contract_data_entry_size(&stored)?; + p.set(key, &Bytes::from_slice(self.env, &stored)); + for page in 0..previous_page_count { + p.remove(&self.blob_page_key(key, page)); + } + return Ok(()); + } + + let page_count = bytes.len().div_ceil(BLOB_PAGE_BYTES) as u32; + let manifest = encode_blob_manifest(bytes.len(), page_count); + ensure_contract_data_entry_size(&manifest)?; + for (page, chunk) in bytes.chunks(BLOB_PAGE_BYTES).enumerate() { + let page_key = self.blob_page_key(key, page as u32); + p.set(&page_key, &Bytes::from_slice(self.env, chunk)); + } + p.set(key, &Bytes::from_slice(self.env, &manifest)); + for page in page_count..previous_page_count { + p.remove(&self.blob_page_key(key, page)); + } + Ok(()) + } + + fn clear_blob(&self, key: &Symbol) { + let page_count = match self.stored_blob_manifest(key) { + Ok(Some(BlobManifest::Paged { page_count, .. })) => page_count, + _ => 0, + }; + let p = self.env.storage().persistent(); + p.remove(key); + for page in 0..page_count { + p.remove(&self.blob_page_key(key, page)); + } + } + + fn extend_blob_ttl(&self, key: &Symbol, threshold: u32, extend_to: u32) { + let p = self.env.storage().persistent(); + if !p.has(key) { + return; + } + p.extend_ttl(key, threshold, extend_to); + if let Ok(Some(BlobManifest::Paged { page_count, .. })) = self.stored_blob_manifest(key) { + for page in 0..page_count { + let page_key = self.blob_page_key(key, page); + if p.has(&page_key) { + p.extend_ttl(&page_key, threshold, extend_to); + } + } + } + } + + fn load_withdraw_queue_page( + &self, + page: u64, + ) -> Result>, RuntimeError> { + let key = self.withdraw_queue_page_key(page); self.env .storage() .persistent() - .set(key, &Bytes::from_slice(self.env, bytes)); + .get::<_, Bytes>(&key) + .map(|bytes| decode_withdraw_queue_page(&bytes.to_alloc_vec())) + .transpose() + } + + fn load_withdraw_queue_pages( + &self, + header: WithdrawQueueHeader, + ) -> Result { + let Some((first_page, last_page)) = queue_header_page_range(header) else { + return Ok(WithdrawQueue::with_state( + Vec::<(u64, PendingWithdrawal)>::new(), + header.next_pending_withdrawal_id, + header.next_pending_withdrawal_id, + )); + }; + + let mut entries = Vec::new(); + let mut last_id = None; + for page in first_page..=last_page { + if let Some(page_entries) = self.load_withdraw_queue_page(page)? { + for (id, _) in &page_entries { + if queue_page_id(*id) != page + || *id < header.next_withdraw_to_execute + || *id >= header.next_pending_withdrawal_id + || last_id.is_some_and(|last| last >= *id) + { + return Err(RuntimeError::storage_error( + "withdraw queue page entries invalid", + )); + } + last_id = Some(*id); + } + entries.extend(page_entries); + } + } + Ok(WithdrawQueue::with_state( + entries, + header.next_withdraw_to_execute, + header.next_pending_withdrawal_id, + )) + } + + fn save_withdraw_queue_pages(&self, queue: &WithdrawQueue) -> Result<(), RuntimeError> { + let previous_range = self + .load_state_blob()? + .and_then(|stored| decode_state_header_blob(&stored).ok()) + .and_then(|header| queue_header_page_range(header.withdraw_queue)); + + let current_range = queue_page_range(queue); + let p = self.env.storage().persistent(); + + if let Some((first_page, last_page)) = current_range { + for page in first_page..=last_page { + let entries = queue + .iter() + .filter(|(id, _)| queue_page_id(*id) == page) + .collect::>(); + let key = self.withdraw_queue_page_key(page); + if entries.is_empty() { + p.remove(&key); + continue; + } + let encoded = encode_withdraw_queue_page(entries); + ensure_contract_data_entry_size(&encoded)?; + let current = p.get::<_, Bytes>(&key).map(|bytes| bytes.to_alloc_vec()); + if current.as_deref() != Some(encoded.as_slice()) { + p.set(&key, &Bytes::from_slice(self.env, &encoded)); + } + } + } + + if let Some((first_page, last_page)) = previous_range { + for page in first_page..=last_page { + let still_live = current_range + .map(|(first, last)| page >= first && page <= last) + .unwrap_or(false); + if !still_live { + p.remove(&self.withdraw_queue_page_key(page)); + } + } + } + + Ok(()) + } + + fn extend_withdraw_queue_page_ttls( + &self, + header: WithdrawQueueHeader, + threshold: u32, + extend_to: u32, + ) { + let Some((first_page, last_page)) = queue_header_page_range(header) else { + return; + }; + let p = self.env.storage().persistent(); + for page in first_page..=last_page { + let key = self.withdraw_queue_page_key(page); + if p.has(&key) { + p.extend_ttl(&key, threshold, extend_to); + } + } } /// Load a kernel-to-Soroban address mapping from persistent storage. @@ -879,70 +1363,67 @@ impl<'a> SorobanStorage<'a> { } } - pub(crate) fn load_state_blob(&self) -> Option> { + pub(crate) fn load_state_blob(&self) -> Result>, RuntimeError> { self.load_blob(&SorobanStorageKey::StateBlob) } - pub(crate) fn save_state_blob(&self, state: &[u8]) { - self.save_blob(&SorobanStorageKey::StateBlob, state); + pub(crate) fn save_state_blob(&self, state: &[u8]) -> Result<(), RuntimeError> { + self.save_blob(&SorobanStorageKey::StateBlob, state) } - pub fn load_policy_locks(&self) -> Option> { + pub fn load_policy_locks(&self) -> Result>, RuntimeError> { self.load_blob(&SorobanStorageKey::PolicyLocks) } - pub fn save_policy_locks(&self, state: &[u8]) { - self.save_blob(&SorobanStorageKey::PolicyLocks, state); + pub fn save_policy_locks(&self, state: &[u8]) -> Result<(), RuntimeError> { + self.save_blob(&SorobanStorageKey::PolicyLocks, state) } - pub fn load_policy_supply_queue(&self) -> Option> { + pub fn load_policy_supply_queue(&self) -> Result>, RuntimeError> { self.load_blob(&SorobanStorageKey::PolicySupplyQueue) } - pub fn save_policy_supply_queue(&self, state: &[u8]) { - self.save_blob(&SorobanStorageKey::PolicySupplyQueue, state); + pub fn save_policy_supply_queue(&self, state: &[u8]) -> Result<(), RuntimeError> { + self.save_blob(&SorobanStorageKey::PolicySupplyQueue, state) } - pub fn load_policy_markets(&self) -> Option> { + pub fn load_policy_markets(&self) -> Result>, RuntimeError> { self.load_blob(&SorobanStorageKey::PolicyMarkets) } - pub fn save_policy_markets(&self, state: &[u8]) { - self.save_blob(&SorobanStorageKey::PolicyMarkets, state); + pub fn save_policy_markets(&self, state: &[u8]) -> Result<(), RuntimeError> { + self.save_blob(&SorobanStorageKey::PolicyMarkets, state) } - pub fn load_policy_principals(&self) -> Option> { + pub fn load_policy_principals(&self) -> Result>, RuntimeError> { self.load_blob(&SorobanStorageKey::PolicyPrincipals) } - pub fn save_policy_principals(&self, state: &[u8]) { - self.save_blob(&SorobanStorageKey::PolicyPrincipals, state); + pub fn save_policy_principals(&self, state: &[u8]) -> Result<(), RuntimeError> { + self.save_blob(&SorobanStorageKey::PolicyPrincipals, state) } - pub fn load_policy_cap_groups(&self) -> Option> { + pub fn load_policy_cap_groups(&self) -> Result>, RuntimeError> { self.load_blob(&SorobanStorageKey::PolicyCapGroups) } - pub fn save_policy_cap_groups(&self, state: &[u8]) { - self.save_blob(&SorobanStorageKey::PolicyCapGroups, state); + pub fn save_policy_cap_groups(&self, state: &[u8]) -> Result<(), RuntimeError> { + self.save_blob(&SorobanStorageKey::PolicyCapGroups, state) } /// Load restrictions from persistent storage. - pub fn load_restrictions(&self) -> Option> { + pub fn load_restrictions(&self) -> Result>, RuntimeError> { self.load_blob(&SorobanStorageKey::Restrictions) } /// Save restrictions to persistent storage. - pub fn save_restrictions(&self, restrictions: &[u8]) { - self.save_blob(&SorobanStorageKey::Restrictions, restrictions); + pub fn save_restrictions(&self, restrictions: &[u8]) -> Result<(), RuntimeError> { + self.save_blob(&SorobanStorageKey::Restrictions, restrictions) } /// Clear restrictions from persistent storage. pub fn clear_restrictions(&self) { - self.env - .storage() - .persistent() - .remove(&SorobanStorageKey::Restrictions); + self.clear_blob(&SorobanStorageKey::Restrictions); } /// Check if the contract is paused. @@ -1005,7 +1486,6 @@ impl<'a> SorobanStorage<'a> { .storage() .instance() .extend_ttl(threshold, extend_to); - let p = self.env.storage().persistent(); // Extend each persistent key if it exists. for key in &[ SorobanStorageKey::StateBlob, @@ -1016,13 +1496,16 @@ impl<'a> SorobanStorage<'a> { SorobanStorageKey::PolicyCapGroups, SorobanStorageKey::Restrictions, ] { - if p.has(key) { - p.extend_ttl(key, threshold, extend_to); - } + self.extend_blob_ttl(key, threshold, extend_to); } - if let Some(stored) = self.load_state_blob() { - if let Ok(state) = decode_state_blob(&stored) { - self.extend_state_address_ttls(&state, threshold, extend_to); + if let Ok(Some(stored)) = self.load_state_blob() { + if let Ok(header) = decode_state_header_blob(&stored) { + self.extend_withdraw_queue_page_ttls(header.withdraw_queue, threshold, extend_to); + if let Ok(queue) = self.load_withdraw_queue_pages(header.withdraw_queue) { + if let Ok(state) = compose_state_from_header(header, queue) { + self.extend_state_address_ttls(&state, threshold, extend_to); + } + } } } } @@ -1034,8 +1517,10 @@ impl<'a> SorobanStorage<'a> { impl Storage for SorobanStorage<'_> { fn load_state(&self) -> Result, RuntimeError> { - if let Some(stored) = self.load_state_blob() { - return Ok(Some(decode_state_blob(&stored)?)); + if let Some(stored) = self.load_state_blob()? { + let header = decode_state_header_blob(&stored)?; + let withdraw_queue = self.load_withdraw_queue_pages(header.withdraw_queue)?; + return Ok(Some(compose_state_from_header(header, withdraw_queue)?)); } Ok(None) @@ -1050,9 +1535,9 @@ impl Storage for SorobanStorage<'_> { "withdraw queue exceeds soroban cap", )); } - let state_blob = encode_state_blob(state); - ensure_contract_data_entry_size(&state_blob)?; - self.save_state_blob(&state_blob); + self.save_withdraw_queue_pages(&state.withdraw_queue)?; + let state_blob = encode_state_header_blob(state); + self.save_state_blob(&state_blob)?; self.extend_default_ttl(); Ok(()) } @@ -1072,23 +1557,23 @@ impl Storage for SorobanStorage<'_> { } fn load_policy_state(&self) -> Result, RuntimeError> { - let leases = match self.load_policy_locks() { + let leases = match self.load_policy_locks()? { Some(stored) => Some(decode_policy_locks(&stored)?), None => None, }; - let supply_queue = match self.load_policy_supply_queue() { + let supply_queue = match self.load_policy_supply_queue()? { Some(stored) => Some(decode_supply_queue(&stored)?), None => None, }; - let markets = match self.load_policy_markets() { + let markets = match self.load_policy_markets()? { Some(stored) => Some(decode_markets(&stored)?), None => None, }; - let principals = match self.load_policy_principals() { + let principals = match self.load_policy_principals()? { Some(stored) => Some(decode_principals(&stored)?), None => None, }; - let cap_groups = match self.load_policy_cap_groups() { + let cap_groups = match self.load_policy_cap_groups()? { Some(stored) => Some(decode_cap_groups(&stored)?), None => None, }; @@ -1102,20 +1587,17 @@ impl Storage for SorobanStorage<'_> { let markets = encode_markets(state.markets()); let principals = encode_principals(state.principals()); let cap_groups = encode_cap_groups(state.cap_groups()); - for bytes in [&locks, &supply_queue, &markets, &principals, &cap_groups] { - ensure_contract_data_entry_size(bytes)?; - } - self.save_policy_locks(&locks); - self.save_policy_supply_queue(&supply_queue); - self.save_policy_markets(&markets); - self.save_policy_principals(&principals); - self.save_policy_cap_groups(&cap_groups); + self.save_policy_locks(&locks)?; + self.save_policy_supply_queue(&supply_queue)?; + self.save_policy_markets(&markets)?; + self.save_policy_principals(&principals)?; + self.save_policy_cap_groups(&cap_groups)?; self.extend_default_ttl(); Ok(()) } fn load_restrictions(&self) -> Result, RuntimeError> { - let restrictions = match SorobanStorage::load_restrictions(self) { + let restrictions = match SorobanStorage::load_restrictions(self)? { Some(stored) => Some(decode_restrictions(&stored)?), None => None, }; @@ -1137,8 +1619,7 @@ impl Storage for SorobanStorage<'_> { return Err(RuntimeError::storage_error("restrictions too large")); } let bytes = encode_restrictions(restrictions); - ensure_contract_data_entry_size(&bytes)?; - SorobanStorage::save_restrictions(self, &bytes); + SorobanStorage::save_restrictions(self, &bytes)?; } else { SorobanStorage::clear_restrictions(self); } diff --git a/contract/vault/soroban/src/tests.rs b/contract/vault/soroban/src/tests.rs index 889dd592e..d604d9881 100644 --- a/contract/vault/soroban/src/tests.rs +++ b/contract/vault/soroban/src/tests.rs @@ -2121,7 +2121,7 @@ mod storage_tests { // Fresh storage should not be initialized assert!(!storage.is_initialized()); - assert!(storage.load_state_blob().is_none()); + assert!(storage.load_state_blob().unwrap().is_none()); // Save state let kernel = VaultState { @@ -2278,7 +2278,9 @@ mod storage_tests { let (env, contract_id) = contract_env; env.as_contract(&contract_id, || { let storage = SorobanStorage::new(&env); - storage.save_state_blob(&alloc::vec![1, 2, 3, 4, 5]); + storage + .save_state_blob(&alloc::vec![1, 2, 3, 4, 5]) + .unwrap(); let err = Storage::load_state(&storage).unwrap_err(); assert_eq!(err, RuntimeError::StorageError); @@ -2384,17 +2386,16 @@ mod storage_tests { ) .unwrap(); - let storage = SorobanStorage::new(&env); - let versioned = fuzz_api::encode_state_blob_bytes(&VaultState::default()); - storage.save_state_blob(&versioned); + let mut storage = SorobanStorage::new(&env); + let state = state_with_pending_withdrawals(2); + Storage::save_state(&mut storage, &state).unwrap(); env.storage() .instance() .set(&soroban_sdk::symbol_short!("migrate"), &true); SorobanVaultContract::migrate(env.clone(), governance).unwrap(); - let stored = storage.load_state_blob().expect("rewritten state blob"); - assert_eq!(stored, versioned); + assert_eq!(Storage::load_state(&storage).unwrap(), Some(state)); assert_eq!( env.storage() .instance() @@ -2420,18 +2421,36 @@ mod storage_tests { env.as_contract(&contract_id, || { let mut storage = SorobanStorage::new(&env); let safe = state_with_pending_withdrawals(SOROBAN_MAX_PENDING_WITHDRAWALS); - assert!(fuzz_api::encode_state_blob_bytes(&safe).len() < 64 * 1024); + assert!(fuzz_api::encode_state_blob_bytes(&safe).len() > 64 * 1024); storage.save_state(&safe).expect("safe queue cap persists"); + assert_eq!(Storage::load_state(&storage).unwrap(), Some(safe)); let too_large = state_with_pending_withdrawals(SOROBAN_MAX_PENDING_WITHDRAWALS + 1); - assert!(fuzz_api::encode_state_blob_bytes(&too_large).len() < 64 * 1024); assert_eq!( storage.save_state(&too_large).unwrap_err(), RuntimeError::StorageError ); + }); + } + + #[rstest] + fn soroban_storage_rejects_missing_withdraw_queue_page( + contract_env: (Env, soroban_sdk::Address), + ) { + let (env, contract_id) = contract_env; + env.as_contract(&contract_id, || { + let mut storage = SorobanStorage::new(&env); + let state = state_with_pending_withdrawals(129); + storage.save_state(&state).expect("paged queue persists"); + + env.storage() + .persistent() + .remove(&(soroban_sdk::symbol_short!("wqpage"), 1u64)); - let kernel_max = state_with_pending_withdrawals(1024); - assert!(fuzz_api::encode_state_blob_bytes(&kernel_max).len() > 64 * 1024); + assert_eq!( + Storage::load_state(&storage).unwrap_err(), + RuntimeError::StorageError + ); }); } @@ -2458,14 +2477,28 @@ mod storage_tests { let (env, contract_id) = contract_env; env.as_contract(&contract_id, || { let mut storage = SorobanStorage::new(&env); - let addresses = (0..=SOROBAN_MAX_RESTRICTION_ADDRESSES) + let addresses = (0..SOROBAN_MAX_RESTRICTION_ADDRESSES) + .map(|i| { + let mut raw = [0u8; 32]; + raw[..8].copy_from_slice(&(i as u64).to_le_bytes()); + KernelAddress(raw) + }) + .collect::>(); + let restrictions = Some(Restrictions::blacklist(addresses.clone())); + Storage::save_restrictions(&mut storage, &restrictions).unwrap(); + assert_eq!( + Storage::load_restrictions(&storage).unwrap(), + Some(Restrictions::blacklist(addresses)) + ); + + let oversized = (0..=SOROBAN_MAX_RESTRICTION_ADDRESSES) .map(|i| { let mut raw = [0u8; 32]; raw[..8].copy_from_slice(&(i as u64).to_le_bytes()); KernelAddress(raw) }) .collect::>(); - let restrictions = Some(Restrictions::blacklist(addresses)); + let restrictions = Some(Restrictions::blacklist(oversized)); assert_eq!( Storage::save_restrictions(&mut storage, &restrictions).unwrap_err(), @@ -2578,7 +2611,9 @@ mod storage_tests { let storage = SorobanStorage::new(&env); let mut markets = OrderedMap::new(); let _ = markets.insert(1, MarketConfig::new(true, 100, None)); - storage.save_policy_markets(&fuzz_api::encode_markets_bytes(&markets)); + storage + .save_policy_markets(&fuzz_api::encode_markets_bytes(&markets)) + .unwrap(); assert_eq!( Storage::load_policy_state(&storage).unwrap_err(), @@ -2765,7 +2800,7 @@ mod storage_tests { let storage = SorobanStorage::new(&env); let mut bytes = fuzz_api::encode_state_blob_bytes(&VaultState::default()); bytes.push(0xff); - storage.save_state_blob(&bytes); + storage.save_state_blob(&bytes).unwrap(); let err = Storage::load_state(&storage).unwrap_err(); assert_eq!(err, RuntimeError::StorageError); From 2c806e7a7e9e2aa6d78d03434b922cccece6359b Mon Sep 17 00:00:00 2001 From: carrion256 Date: Fri, 22 May 2026 11:58:54 +0200 Subject: [PATCH 4/4] fix: bound Soroban storage decoder allocations (#FIND-067 #FIND-101 Nexus f37692f9-99d1-466c-afdd-076b14f35bbb Nexus afb88ec4-85a4-487b-890c-63e360bff8f6) Harden versioned storage decoders so malformed length prefixes cannot drive oversized Vec preallocation before the remaining byte budget is validated. Also removes the unused Env parameter from AddressMap::new and updates callers/tests. Verification: cargo fmt --all; CARGO_INCREMENTAL=0 CARGO_TARGET_DIR=/data/tmp/contracts-storage-a067-target cargo test -p templar-soroban-runtime storage_codec_rejects_malformed -- --nocapture; CARGO_INCREMENTAL=0 CARGO_TARGET_DIR=/data/tmp/contracts-storage-a067-target cargo test -p templar-soroban-runtime test_address_map -- --nocapture; git diff --check. --- contract/vault/soroban/src/effects/mod.rs | 4 +- contract/vault/soroban/src/storage/mod.rs | 33 +++++++++++++- contract/vault/soroban/src/tests.rs | 53 ++++++++++++++++++++++- 3 files changed, 86 insertions(+), 4 deletions(-) diff --git a/contract/vault/soroban/src/effects/mod.rs b/contract/vault/soroban/src/effects/mod.rs index f48d6a3aa..832315940 100644 --- a/contract/vault/soroban/src/effects/mod.rs +++ b/contract/vault/soroban/src/effects/mod.rs @@ -584,7 +584,7 @@ impl AddressMap { /// Create a new address map. #[inline] #[must_use] - pub fn new(_env: &Env) -> Self { + pub fn new() -> Self { Self { addresses: AddressBook::new(), } @@ -642,7 +642,7 @@ where env, share_token, asset_token, - address_map: AddressMap::new(env), + address_map: AddressMap::new(), } } diff --git a/contract/vault/soroban/src/storage/mod.rs b/contract/vault/soroban/src/storage/mod.rs index d87a2a85e..10bede94a 100644 --- a/contract/vault/soroban/src/storage/mod.rs +++ b/contract/vault/soroban/src/storage/mod.rs @@ -287,6 +287,23 @@ fn read_bytes<'a>(bytes: &'a [u8], cursor: &mut usize) -> Result<&'a [u8], Runti read_exact(bytes, cursor, len) } +fn bounded_count_for_fixed_items( + bytes: &[u8], + cursor: usize, + count: usize, + item_size: usize, + error: &'static str, +) -> Result { + let remaining = bytes + .len() + .checked_sub(cursor) + .ok_or_else(|| RuntimeError::storage_error(error))?; + if count > remaining / item_size { + return Err(RuntimeError::storage_error(error)); + } + Ok(count) +} + fn finish_decode(bytes: &[u8], cursor: usize) -> Result<(), RuntimeError> { if cursor == bytes.len() { Ok(()) @@ -382,6 +399,7 @@ fn decode_supply_queue_v1(bytes: &[u8]) -> Result { let mut cursor = 0usize; let max_length = read_u32(bytes, &mut cursor)?; let count = read_u32(bytes, &mut cursor)? as usize; + let count = bounded_count_for_fixed_items(bytes, cursor, count, 21, "supply queue too large")?; let mut entries = Vec::with_capacity(count); for _ in 0..count { let target_id = read_u32(bytes, &mut cursor)?; @@ -645,6 +663,8 @@ fn decode_withdraw_queue(bytes: &[u8], cursor: &mut usize) -> Result SOROBAN_MAX_PENDING_WITHDRAWALS as usize { return Err(RuntimeError::storage_error("withdraw queue too large")); } + let count = + bounded_count_for_fixed_items(bytes, *cursor, count, 112, "withdraw queue too large")?; if next_withdraw_to_execute > next_pending_withdrawal_id { return Err(RuntimeError::storage_error("withdraw queue invalid ids")); } @@ -800,6 +820,8 @@ fn decode_withdraw_queue_page(bytes: &[u8]) -> Result WITHDRAW_QUEUE_PAGE_SIZE as usize { return Err(RuntimeError::storage_error("withdraw queue page too large")); } + let count = + bounded_count_for_fixed_items(bytes, cursor, count, 112, "withdraw queue page too large")?; let mut entries = Vec::with_capacity(count); for _ in 0..count { let id = read_u64(bytes, &mut cursor)?; @@ -871,6 +893,13 @@ fn decode_op_state(bytes: &[u8], cursor: &mut usize) -> Result Result SorobanStorage<'a> { Ok(Some(stored[cursor..cursor + len].to_vec())) } Some(BlobManifest::Paged { len, page_count }) => { - let mut out = Vec::with_capacity(len); + let mut out = Vec::with_capacity(len.min(BLOB_PAGE_BYTES)); let p = self.env.storage().persistent(); for page in 0..page_count { let page_key = self.blob_page_key(key, page); diff --git a/contract/vault/soroban/src/tests.rs b/contract/vault/soroban/src/tests.rs index d604d9881..20d3ac38f 100644 --- a/contract/vault/soroban/src/tests.rs +++ b/contract/vault/soroban/src/tests.rs @@ -1698,7 +1698,7 @@ mod effects_tests { #[test] fn test_address_map() { let env = test_env(); - let mut map = AddressMap::new(&env); + let mut map = AddressMap::new(); let kernel_addr = templar_vault_kernel::Address([1u8; 32]); let soroban_addr = Address::generate(&env); @@ -2524,6 +2524,57 @@ mod storage_tests { } } + fn versioned_storage_bytes(kind: u8, payload: &[u8]) -> alloc::vec::Vec { + let mut bytes = alloc::vec::Vec::with_capacity(5 + payload.len()); + bytes.extend_from_slice(b"TVS"); + bytes.push(kind); + bytes.push(1); + bytes.extend_from_slice(payload); + bytes + } + + #[rstest] + #[case::supply_queue_entries(u32::MAX.to_le_bytes().to_vec())] + #[case::supply_queue_max_plus_entries({ + let mut payload = alloc::vec::Vec::new(); + payload.extend_from_slice(&1u32.to_le_bytes()); + payload.extend_from_slice(&u32::MAX.to_le_bytes()); + payload + })] + fn storage_codec_rejects_malformed_supply_queue_lengths(#[case] payload: alloc::vec::Vec) { + let encoded = versioned_storage_bytes(3, &payload); + assert!(fuzz_api::decode_supply_queue_bytes(&encoded).is_err()); + } + + #[rstest] + #[case::allocating_plan(1u8, 20usize, "allocating")] + #[case::refreshing_plan(3u8, 4usize, "refreshing")] + fn storage_codec_rejects_malformed_op_state_plan_lengths( + #[case] tag: u8, + #[case] item_size: usize, + #[case] name: &str, + ) { + let mut payload = alloc::vec::Vec::new(); + for _ in 0..5 { + payload.extend_from_slice(&0u128.to_le_bytes()); + } + payload.extend_from_slice(&0u64.to_le_bytes()); + payload.push(tag); + payload.extend_from_slice(&7u64.to_le_bytes()); + payload.extend_from_slice(&0u32.to_le_bytes()); + if tag == 1 { + payload.extend_from_slice(&0u128.to_le_bytes()); + } + payload.extend_from_slice(&u32::MAX.to_le_bytes()); + payload.extend_from_slice(&alloc::vec![0u8; item_size - 1]); + + let encoded = versioned_storage_bytes(1, &payload); + assert!( + fuzz_api::decode_state_blob_bytes(&encoded).is_err(), + "{name} oversized plan length must fail before preallocating" + ); + } + #[test] fn storage_codec_roundtrip_supply_queue_and_truncated_bytes_fail_cleanly() { let queue =