From ba51b37f3b02298b6496465f35f286cf970df7e5 Mon Sep 17 00:00:00 2001 From: carrion256 Date: Mon, 18 May 2026 17:29:32 +0200 Subject: [PATCH 1/4] fix: validate runtime role address forms #FIND-044 --- .../vault/soroban/src/contract/entrypoints.rs | 26 +++- .../vault/soroban/src/contract/helpers.rs | 12 +- contract/vault/soroban/src/contract/mod.rs | 2 +- contract/vault/soroban/src/tests.rs | 137 ++++++++++-------- .../vault/soroban/tests/integration_tests.rs | 77 +++++++++- 5 files changed, 185 insertions(+), 69 deletions(-) diff --git a/contract/vault/soroban/src/contract/entrypoints.rs b/contract/vault/soroban/src/contract/entrypoints.rs index 61f4e5b8d..762866da1 100644 --- a/contract/vault/soroban/src/contract/entrypoints.rs +++ b/contract/vault/soroban/src/contract/entrypoints.rs @@ -9,8 +9,9 @@ use super::helpers::{ ensure_governance_identity, ensure_sentinel_identity, extend_storage_ttl, get_config_address, governance_caller, kernel_address_from_sdk, load_virtual_offsets, migrate_legacy_paused, migration_in_progress, require_contract_address, require_governance, - require_governance_control_plane, require_sentinel, require_signed, sdk_string_to_alloc, - set_config_address, set_migration_in_progress, store_fees_spec, store_virtual_offsets, + require_governance_control_plane, require_sentinel, require_signed, + require_wasm_or_account_address, sdk_string_to_alloc, set_config_address, + set_migration_in_progress, store_fees_spec, store_virtual_offsets, with_contract_vault_contract_error, }; use super::*; @@ -57,9 +58,11 @@ fn require_unique_addresses( Ok(()) } -fn apply_curator_config(env: &Env, new_curator: soroban_sdk::Address) { +fn apply_curator_config(env: &Env, new_curator: soroban_sdk::Address) -> Result<(), ContractError> { + require_wasm_or_account_address(&new_curator)?; set_config_address(env, &VaultDataKey::Curator, &new_curator); emit_admin_event(env, symbol_short!("s_curatr")); + Ok(()) } fn apply_governance_config( @@ -72,17 +75,22 @@ fn apply_governance_config( Ok(()) } -fn apply_sentinel_config(env: &Env, sentinel: soroban_sdk::Address) { +fn apply_sentinel_config(env: &Env, sentinel: soroban_sdk::Address) -> Result<(), ContractError> { + require_wasm_or_account_address(&sentinel)?; env.storage() .instance() .set(&VaultDataKey::Sentinel, &sentinel); emit_admin_event(env, symbol_short!("s_sntnl")); + Ok(()) } fn apply_guardians_config( env: &Env, guardians: soroban_sdk::Vec, ) -> Result<(), ContractError> { + for guardian in guardians.iter() { + require_wasm_or_account_address(&guardian)?; + } require_unique_addresses(&guardians)?; env.storage() .instance() @@ -95,6 +103,9 @@ fn apply_allocators_config( env: &Env, allocators: soroban_sdk::Vec, ) -> Result<(), ContractError> { + for allocator in allocators.iter() { + require_wasm_or_account_address(&allocator)?; + } require_unique_addresses(&allocators)?; env.storage() .instance() @@ -580,11 +591,11 @@ fn set_governance_config_impl( value_b: Option, ) -> Result<(), ContractError> { match kind { - GOVERNANCE_CONFIG_KIND_CURATOR => apply_curator_config(env, required_address(primary)?), + GOVERNANCE_CONFIG_KIND_CURATOR => apply_curator_config(env, required_address(primary)?)?, GOVERNANCE_CONFIG_KIND_GOVERNANCE => { apply_governance_config(env, required_address(primary)?)? } - GOVERNANCE_CONFIG_KIND_SENTINEL => apply_sentinel_config(env, required_address(primary)?), + GOVERNANCE_CONFIG_KIND_SENTINEL => apply_sentinel_config(env, required_address(primary)?)?, GOVERNANCE_CONFIG_KIND_GUARDIANS => apply_guardians_config(env, required_addresses(many)?)?, GOVERNANCE_CONFIG_KIND_ALLOCATORS => { apply_allocators_config(env, required_addresses(many)?)? @@ -919,6 +930,9 @@ impl SorobanVaultContract { let virtual_shares = to_u128(virtual_shares)?; let virtual_assets = to_u128(virtual_assets)?; + require_wasm_or_account_address(&curator)?; + require_contract_address(&governance)?; + set_config_address(&env, &VaultDataKey::Curator, &curator); set_config_address(&env, &VaultDataKey::Governance, &governance); set_config_address(&env, &VaultDataKey::AssetToken, &asset_token); diff --git a/contract/vault/soroban/src/contract/helpers.rs b/contract/vault/soroban/src/contract/helpers.rs index 30d466ae3..1d112e418 100644 --- a/contract/vault/soroban/src/contract/helpers.rs +++ b/contract/vault/soroban/src/contract/helpers.rs @@ -103,8 +103,10 @@ pub(crate) fn addresses_from_alloc_strings( } fn is_contract_address(addr: &SdkAddress) -> bool { - let bytes = addr.to_string().to_bytes(); - matches!(bytes.get(0), Some(b'C')) + matches!( + addr.executable(), + Some(Executable::Wasm(_)) | Some(Executable::StellarAsset) + ) } pub(crate) fn require_contract_address(addr: &SdkAddress) -> Result<(), ContractError> { @@ -113,6 +115,12 @@ pub(crate) fn require_contract_address(addr: &SdkAddress) -> Result<(), Contract .ok_or(ContractError::InvalidInput) } +pub(crate) fn require_wasm_or_account_address(addr: &SdkAddress) -> Result<(), ContractError> { + (!matches!(addr.executable(), Some(Executable::StellarAsset))) + .then_some(()) + .ok_or(ContractError::InvalidInput) +} + fn allowed_adapters(env: &Env) -> Option> { env.storage().instance().get(&VaultDataKey::AllowedAdapters) } diff --git a/contract/vault/soroban/src/contract/mod.rs b/contract/vault/soroban/src/contract/mod.rs index becb9342a..ddd0cc84d 100644 --- a/contract/vault/soroban/src/contract/mod.rs +++ b/contract/vault/soroban/src/contract/mod.rs @@ -35,7 +35,7 @@ use alloc::vec::Vec; use core::mem; pub(crate) use helpers::*; use soroban_sdk::{ - contract, contractimpl, symbol_short, Address as SdkAddress, Bytes, BytesN, Env, + contract, contractimpl, symbol_short, Address as SdkAddress, Bytes, BytesN, Env, Executable, }; use templar_curator_primitives::governance::TimelockDecision; use templar_curator_primitives::policy::cap_group::{CapGroupId, CapGroupRecord, CapGroupUpdate}; diff --git a/contract/vault/soroban/src/tests.rs b/contract/vault/soroban/src/tests.rs index c12582c84..353839dba 100644 --- a/contract/vault/soroban/src/tests.rs +++ b/contract/vault/soroban/src/tests.rs @@ -295,8 +295,10 @@ mod contract_tests { use alloc::string::{String as AllocString, ToString}; use alloc::vec; use alloc::vec::Vec; + use soroban_sdk::testutils::Address as _; use soroban_sdk::{Address as SdkAddress, Bytes, Env}; use templar_curator_primitives::PolicyState; + use templar_soroban_governance::SorobanVaultGovernanceContract; use templar_soroban_shared_types::{ GovernanceCommand, VaultCommand, VaultCommandResult, GOVERNANCE_CONFIG_KIND_VIRTUAL_OFFSETS, }; @@ -370,6 +372,24 @@ mod contract_tests { .expect("valid address") } + fn register_runtime_contracts( + env: &Env, + contract_id: &SdkAddress, + admin: &SdkAddress, + ) -> (SdkAddress, SdkAddress, SdkAddress) { + let governance = env.register( + SorobanVaultGovernanceContract, + (admin, contract_id, &(0u64)), + ); + let asset = env + .register_stellar_asset_contract_v2(SdkAddress::generate(env)) + .address(); + let share = env + .register_stellar_asset_contract_v2(contract_id.clone()) + .address(); + (governance, asset, share) + } + fn execute_command( env: &Env, command: &VaultCommand, @@ -910,14 +930,13 @@ mod contract_tests { let contract_id = env.register(SorobanVaultContract, ()); let curator = soroban_sdk::Address::generate(&env); - let asset = soroban_sdk::Address::generate(&env); - let share = soroban_sdk::Address::generate(&env); + let (governance, asset, share) = register_runtime_contracts(&env, &contract_id, &curator); env.as_contract(&contract_id, || { SorobanVaultContract::initialize( env.clone(), curator.clone(), - curator, + governance, asset, share, 0, @@ -969,14 +988,13 @@ mod contract_tests { let contract_id = env.register(SorobanVaultContract, ()); let curator = soroban_sdk::Address::generate(&env); - let asset = soroban_sdk::Address::generate(&env); - let share = soroban_sdk::Address::generate(&env); + let (governance, asset, share) = register_runtime_contracts(&env, &contract_id, &curator); env.as_contract(&contract_id, || { SorobanVaultContract::initialize( env.clone(), curator.clone(), - curator.clone(), + governance, asset, share, 17, @@ -1002,9 +1020,7 @@ mod contract_tests { let contract_id = env.register(SorobanVaultContract, ()); let curator = soroban_sdk::Address::generate(&env); - let governance = soroban_sdk::Address::generate(&env); - let asset = soroban_sdk::Address::generate(&env); - let share = soroban_sdk::Address::generate(&env); + let (governance, asset, share) = register_runtime_contracts(&env, &contract_id, &curator); env.as_contract(&contract_id, || { SorobanVaultContract::initialize( @@ -1253,7 +1269,12 @@ mod contract_tests { env.as_contract(&contract_id, || { proxy - .initialize(curator.clone(), curator, asset.clone(), share.clone()) + .initialize( + curator.clone(), + register_runtime_contracts(&env, &contract_id, &curator).0, + asset.clone(), + share.clone(), + ) .unwrap(); let mut storage = SorobanStorage::new(&env); @@ -1324,7 +1345,7 @@ mod contract_tests { SorobanVaultContract::initialize( env.clone(), curator.clone(), - curator.clone(), + register_runtime_contracts(&env, &contract_id, &curator).0, asset.clone(), share.clone(), 0, @@ -1394,14 +1415,13 @@ mod contract_tests { let contract_id = env.register(SorobanVaultContract, ()); let curator = soroban_sdk::Address::generate(&env); - let asset = soroban_sdk::Address::generate(&env); - let share = soroban_sdk::Address::generate(&env); + let (governance, asset, share) = register_runtime_contracts(&env, &contract_id, &curator); env.as_contract(&contract_id, || { SorobanVaultContract::initialize( env.clone(), curator.clone(), - curator, + governance, asset, share, 0, @@ -1973,6 +1993,7 @@ mod storage_tests { use templar_curator_primitives::policy::cap_group::{CapGroup, CapGroupId, CapGroupRecord}; use templar_curator_primitives::policy::state::{MarketConfig, OrderedMap}; use templar_curator_primitives::PolicyState; + use templar_soroban_governance::SorobanVaultGovernanceContract; use templar_soroban_shared_types::{ GovernanceCommand, GOVERNANCE_CONFIG_KIND_ALLOCATORS, GOVERNANCE_CONFIG_KIND_ALLOWED_ADAPTERS, GOVERNANCE_CONFIG_KIND_CURATOR, @@ -1991,6 +2012,24 @@ mod storage_tests { .expect("valid address") } + fn register_runtime_contracts( + env: &Env, + contract_id: &SdkAddress, + admin: &SdkAddress, + ) -> (SdkAddress, SdkAddress, SdkAddress) { + let governance = env.register( + SorobanVaultGovernanceContract, + (admin, contract_id, &(0u64)), + ); + let asset = env + .register_stellar_asset_contract_v2(SdkAddress::generate(env)) + .address(); + let share = env + .register_stellar_asset_contract_v2(contract_id.clone()) + .address(); + (governance, asset, share) + } + fn execute_governance_command( env: &Env, contract_id: &SdkAddress, @@ -2647,9 +2686,7 @@ mod storage_tests { 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); + let (governance, asset, share) = register_runtime_contracts(&env, &contract_id, &curator); env.as_contract(&contract_id, || { SorobanVaultContract::initialize( @@ -2715,9 +2752,7 @@ mod storage_tests { 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); + let (governance, asset, share) = register_runtime_contracts(&env, &contract_id, &curator); env.as_contract(&contract_id, || { SorobanVaultContract::initialize( @@ -2772,9 +2807,7 @@ mod storage_tests { 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); + let (governance, asset, share) = register_runtime_contracts(&env, &contract_id, &curator); env.as_contract(&contract_id, || { SorobanVaultContract::initialize( @@ -2825,11 +2858,9 @@ mod storage_tests { 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 (governance, asset, share) = register_runtime_contracts(&env, &contract_id, &curator); let sentinel = SdkAddress::generate(&env); let attacker = SdkAddress::generate(&env); - let asset = SdkAddress::generate(&env); - let share = SdkAddress::generate(&env); env.as_contract(&contract_id, || { SorobanVaultContract::initialize( @@ -3018,12 +3049,10 @@ mod storage_tests { 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 (governance, asset, share) = register_runtime_contracts(&env, &contract_id, &curator); let sentinel = SdkAddress::generate(&env); let attacker = SdkAddress::generate(&env); let restricted = SdkAddress::generate(&env); - let asset = SdkAddress::generate(&env); - let share = SdkAddress::generate(&env); env.as_contract(&contract_id, || { SorobanVaultContract::initialize( @@ -3187,9 +3216,7 @@ mod storage_tests { 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); + let (governance, asset, share) = register_runtime_contracts(&env, &contract_id, &curator); let cap_group_id = CapGroupId::try_from("group-c".to_string()).unwrap(); env.as_contract(&contract_id, || { @@ -3273,10 +3300,8 @@ mod storage_tests { 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 (governance, asset, share) = register_runtime_contracts(&env, &contract_id, &curator); let attacker = SdkAddress::generate(&env); - let asset = SdkAddress::generate(&env); - let share = SdkAddress::generate(&env); env.as_contract(&contract_id, || { SorobanVaultContract::initialize(env.clone(), curator, governance, asset, share, 0, 0) @@ -3296,9 +3321,7 @@ mod storage_tests { 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); + let (governance, asset, share) = register_runtime_contracts(&env, &contract_id, &curator); let cap_group_id = CapGroupId::try_from("group-c".to_string()).unwrap(); env.as_contract(&contract_id, || { @@ -3362,9 +3385,7 @@ mod storage_tests { 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); + let (governance, asset, share) = register_runtime_contracts(&env, &contract_id, &curator); env.as_contract(&contract_id, || { SorobanVaultContract::initialize( @@ -3418,11 +3439,21 @@ mod storage_tests { 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); + let governance = env.register( + SorobanVaultGovernanceContract, + (&curator, &contract_id, &(0u64)), + ); + let asset = env + .register_stellar_asset_contract_v2(SdkAddress::generate(&env)) + .address(); + let share = env + .register_stellar_asset_contract_v2(contract_id.clone()) + .address(); let new_curator = SdkAddress::generate(&env); - let new_governance = SdkAddress::generate(&env); + let new_governance = env.register( + SorobanVaultGovernanceContract, + (&curator, &contract_id, &(0u64)), + ); let sentinel = SdkAddress::generate(&env); let guardian = SdkAddress::generate(&env); @@ -3533,9 +3564,7 @@ mod storage_tests { 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); + let (governance, asset, share) = register_runtime_contracts(&env, &contract_id, &curator); env.as_contract(&contract_id, || { SorobanVaultContract::initialize( @@ -3578,9 +3607,7 @@ mod storage_tests { 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); + let (governance, asset, share) = register_runtime_contracts(&env, &contract_id, &curator); let adapter = SdkAddress::generate(&env); env.as_contract(&contract_id, || { @@ -3651,9 +3678,7 @@ mod storage_tests { 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); + let (governance, asset, share) = register_runtime_contracts(&env, &contract_id, &curator); let attacker = SdkAddress::generate(&env); env.as_contract(&contract_id, || { @@ -3741,9 +3766,7 @@ mod storage_tests { 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); + let (governance, asset, share) = register_runtime_contracts(&env, &contract_id, &curator); env.as_contract(&contract_id, || { SorobanVaultContract::initialize( diff --git a/contract/vault/soroban/tests/integration_tests.rs b/contract/vault/soroban/tests/integration_tests.rs index 4c3d85a98..276331056 100644 --- a/contract/vault/soroban/tests/integration_tests.rs +++ b/contract/vault/soroban/tests/integration_tests.rs @@ -6,7 +6,7 @@ use rstest::{fixture, rstest}; use soroban_sdk::{ testutils::{Address as _, Ledger, LedgerInfo}, token::StellarAssetClient, - Bytes, Env, + Address as SdkAddress, Bytes, Env, }; use std::string::String as AllocString; use templar_curator_primitives::policy::state::MarketConfig; @@ -21,7 +21,9 @@ use templar_soroban_runtime::{ Storage, // Import the trait }; use templar_soroban_shared_types::{ - GovernanceCommand, VaultCommand, VaultCommandResult, GOVERNANCE_CONFIG_KIND_VIRTUAL_OFFSETS, + GovernanceCommand, VaultCommand, VaultCommandResult, GOVERNANCE_CONFIG_KIND_ALLOCATORS, + GOVERNANCE_CONFIG_KIND_CURATOR, GOVERNANCE_CONFIG_KIND_GUARDIANS, + GOVERNANCE_CONFIG_KIND_SENTINEL, GOVERNANCE_CONFIG_KIND_VIRTUAL_OFFSETS, }; use templar_vault_kernel::state::queue::DEFAULT_COOLDOWN_NS; use templar_vault_kernel::{ @@ -218,15 +220,84 @@ fn soroban_contract_fixture() -> SorobanContractFixture { let share = env .register_stellar_asset_contract_v2(contract_id.clone()) .address(); + let governance = env.register( + SorobanVaultGovernanceContract, + (&curator, &contract_id, &(0u64)), + ); env.as_contract(&contract_id, || { - SorobanVaultContract::initialize(env.clone(), curator.clone(), curator, asset, share, 0, 0) + SorobanVaultContract::initialize(env.clone(), curator, governance, asset, share, 0, 0) .unwrap(); }); SorobanContractFixture { env, contract_id } } +#[test] +fn runtime_initialize_rejects_non_contract_governance() { + let env = Env::default(); + env.mock_all_auths(); + let vault = env.register(SorobanVaultContract, ()); + let curator = soroban_sdk::Address::generate(&env); + let governance = SdkAddress::from_str( + &env, + "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF", + ); + let asset = env + .register_stellar_asset_contract_v2(soroban_sdk::Address::generate(&env)) + .address(); + let share = env + .register_stellar_asset_contract_v2(vault.clone()) + .address(); + + let result = env.as_contract(&vault, || { + SorobanVaultContract::initialize(env.clone(), curator, governance, asset, share, 0, 0) + }); + + assert_eq!( + result, + Err(templar_soroban_runtime::ContractError::InvalidInput) + ); +} + +#[rstest] +#[case(GOVERNANCE_CONFIG_KIND_CURATOR, true)] +#[case(GOVERNANCE_CONFIG_KIND_SENTINEL, true)] +#[case(GOVERNANCE_CONFIG_KIND_GUARDIANS, false)] +#[case(GOVERNANCE_CONFIG_KIND_ALLOCATORS, false)] +fn runtime_governance_config_rejects_sac_role_addresses( + #[case] kind: u32, + #[case] primary_role: bool, + soroban_contract_fixture: SorobanContractFixture, +) { + let env = soroban_contract_fixture.env; + let contract_id = soroban_contract_fixture.contract_id; + let proxy = VaultProxy::new(&env); + let contract_role = env + .register_stellar_asset_contract_v2(contract_id.clone()) + .address(); + + env.as_contract(&contract_id, || { + let governance = proxy.governance().unwrap(); + let many = if primary_role { + None + } else { + Some(std::vec![sdk_wire(&contract_role)]) + }; + let command = GovernanceCommand::SetGovernanceConfig { + kind, + primary: primary_role.then(|| sdk_wire(&contract_role)), + many, + value_a: None, + value_b: None, + }; + assert_eq!( + proxy.execute_governance_unit(&governance, &command), + Err(templar_soroban_runtime::ContractError::InvalidInput) + ); + }); +} + #[rstest] fn soroban_contract_vault_snapshot_matches_fields( soroban_contract_fixture: SorobanContractFixture, From 4d340f78d316e94e427c18ceb45db351ccff5c96 Mon Sep 17 00:00:00 2001 From: carrion256 Date: Mon, 18 May 2026 17:33:08 +0200 Subject: [PATCH 2/4] fix: reject invalid governance targets #FIND-047 --- contract/vault/soroban/governance/src/lib.rs | 32 +++++++++++++++---- .../vault/soroban/governance/src/tests.rs | 25 ++++++++++++++- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/contract/vault/soroban/governance/src/lib.rs b/contract/vault/soroban/governance/src/lib.rs index a205b283f..5dce1dbdb 100644 --- a/contract/vault/soroban/governance/src/lib.rs +++ b/contract/vault/soroban/governance/src/lib.rs @@ -8,7 +8,7 @@ pub use types::*; use alloc::{string::String as AllocString, vec::Vec as AllocVec}; use soroban_sdk::{ auth::{ContractContext, InvokerContractAuthEntry, SubContractInvocation}, - contract, contractimpl, Address, Bytes, BytesN, Env, IntoVal, String, Symbol, Vec, + contract, contractimpl, Address, Bytes, BytesN, Env, Executable, IntoVal, String, Symbol, Vec, }; use templar_curator_primitives::governance::{ timelock_config_decision, CapChangeError, FeeChangeError, FeeConfig, MembershipChangeError, @@ -180,7 +180,7 @@ impl SorobanVaultGovernanceContract { caller: Address, governance: Address, ) -> Result { - require_contract_address(&governance)?; + require_governance_target(&env, &governance)?; Self::submit(env, caller, GovernanceAction::SetGovernance(governance)) } @@ -604,7 +604,7 @@ impl SorobanVaultGovernanceContract { extend_instance_ttl(&env); require_admin(&env, &caller)?; require_not_abdicated(&env, &action)?; - validate_action(&action)?; + validate_action(&env, &action)?; let id = next_proposal_id(&env)?; let decision = decide_submission(&env, &action)?; @@ -717,9 +717,9 @@ fn require_unique_target_ids(target_ids: &Vec) -> Result<(), GovernanceErro Ok(()) } -fn validate_action(action: &GovernanceAction) -> Result<(), GovernanceError> { +fn validate_action(env: &Env, action: &GovernanceAction) -> Result<(), GovernanceError> { match action { - GovernanceAction::SetGovernance(governance) => require_contract_address(governance), + GovernanceAction::SetGovernance(governance) => require_governance_target(env, governance), GovernanceAction::SetFees(params) => { let _ = to_wad(params.performance_fee_wad)?; let _ = to_wad(params.management_fee_wad)?; @@ -1654,8 +1654,10 @@ fn ledger_timestamp_ns(env: &Env) -> Result { } fn is_contract_address(addr: &Address) -> bool { - let bytes = addr.to_string().to_bytes(); - matches!(bytes.get(0), Some(b'C')) + matches!( + addr.executable(), + Some(Executable::Wasm(_)) | Some(Executable::StellarAsset) + ) } fn require_contract_address(addr: &Address) -> Result<(), GovernanceError> { @@ -1666,6 +1668,22 @@ fn require_contract_address(addr: &Address) -> Result<(), GovernanceError> { } } +fn require_wasm_contract_address(addr: &Address) -> Result<(), GovernanceError> { + match addr.executable() { + Some(Executable::Wasm(_)) => Ok(()), + _ => Err(GovernanceError::InvalidInput), + } +} + +fn require_governance_target(env: &Env, governance: &Address) -> Result<(), GovernanceError> { + require_wasm_contract_address(governance)?; + let vault = get_address(env, DataKey::Vault)?; + if governance == &vault || governance == &env.current_contract_address() { + return Err(GovernanceError::InvalidInput); + } + Ok(()) +} + fn extend_instance_ttl(env: &Env) { env.storage() .instance() diff --git a/contract/vault/soroban/governance/src/tests.rs b/contract/vault/soroban/governance/src/tests.rs index f920bc7e5..f41eb1115 100644 --- a/contract/vault/soroban/governance/src/tests.rs +++ b/contract/vault/soroban/governance/src/tests.rs @@ -1269,6 +1269,29 @@ fn cap_group_membership_clear_uses_mirrored_current_membership() { assert_eq!(duplicate_clear, Err(GovernanceError::NoChange)); } +#[test] +fn set_governance_rejects_obvious_invalid_contract_targets() { + let env = Env::default(); + env.mock_all_auths(); + let admin = Address::generate(&env); + let vault = env.register(MockVault, ()); + let governance = env.register(SorobanVaultGovernanceContract, (&admin, &vault, &(0u64))); + let asset_contract = env + .register_stellar_asset_contract_v2(Address::generate(&env)) + .address(); + + for target in [vault.clone(), governance.clone(), asset_contract] { + let result = env.as_contract(&governance, || { + SorobanVaultGovernanceContract::submit_set_governance( + env.clone(), + admin.clone(), + target.clone(), + ) + }); + assert_eq!(result, Err(GovernanceError::InvalidInput)); + } +} + #[test] fn governance_change_is_timelocked_and_routes_to_vault() { let env = Env::default(); @@ -1286,7 +1309,7 @@ fn governance_change_is_timelocked_and_routes_to_vault() { (&admin, &vault, &(5_000_000_000u64)), ); - let new_governance = Address::generate(&env); + let new_governance = env.register(SorobanVaultGovernanceContract, (&admin, &vault, &(0u64))); let proposal_id = env.as_contract(&governance, || { SorobanVaultGovernanceContract::submit_set_governance( From 80c26190f23eef6f2d7754bdc57b9c3e8ce6afc9 Mon Sep 17 00:00:00 2001 From: carrion256 Date: Mon, 18 May 2026 17:36:35 +0200 Subject: [PATCH 3/4] fix: reject governance constructor collisions #FIND-063 --- contract/vault/soroban/governance/src/lib.rs | 15 +++++++ .../vault/soroban/governance/src/tests.rs | 39 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/contract/vault/soroban/governance/src/lib.rs b/contract/vault/soroban/governance/src/lib.rs index 5dce1dbdb..48d7fcbb1 100644 --- a/contract/vault/soroban/governance/src/lib.rs +++ b/contract/vault/soroban/governance/src/lib.rs @@ -1,6 +1,8 @@ #![no_std] extern crate alloc; +#[cfg(test)] +extern crate std; mod types; pub use types::*; @@ -120,6 +122,7 @@ impl SorobanVaultGovernanceContract { ) -> Result<(), GovernanceError> { extend_instance_ttl(&env); require_contract_address(&vault)?; + require_constructor_topology(&env, &admin, &vault)?; validate_timelock_ns(timelock_ns)?; env.storage().instance().set(&DataKey::Admin, &admin); @@ -1684,6 +1687,18 @@ fn require_governance_target(env: &Env, governance: &Address) -> Result<(), Gove Ok(()) } +fn require_constructor_topology( + env: &Env, + admin: &Address, + vault: &Address, +) -> Result<(), GovernanceError> { + let current = env.current_contract_address(); + if admin == vault || admin == ¤t || vault == ¤t { + return Err(GovernanceError::InvalidInput); + } + Ok(()) +} + fn extend_instance_ttl(env: &Env) { env.storage() .instance() diff --git a/contract/vault/soroban/governance/src/tests.rs b/contract/vault/soroban/governance/src/tests.rs index f41eb1115..5d8e38ef8 100644 --- a/contract/vault/soroban/governance/src/tests.rs +++ b/contract/vault/soroban/governance/src/tests.rs @@ -1269,6 +1269,45 @@ fn cap_group_membership_clear_uses_mirrored_current_membership() { assert_eq!(duplicate_clear, Err(GovernanceError::NoChange)); } +#[test] +fn governance_constructor_rejects_self_referential_or_colliding_roles() { + let env = Env::default(); + env.mock_all_auths(); + let admin = Address::generate(&env); + let vault = env.register(MockVault, ()); + + let admin_is_vault = Address::generate(&env); + let self_as_admin = Address::generate(&env); + let self_as_vault = Address::generate(&env); + + let admin_is_vault_result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + env.register_at( + &admin_is_vault, + SorobanVaultGovernanceContract, + (&vault, &vault, &(0u64)), + ); + })); + assert!(admin_is_vault_result.is_err()); + + let self_admin_result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + env.register_at( + &self_as_admin, + SorobanVaultGovernanceContract, + (&self_as_admin, &vault, &(0u64)), + ); + })); + assert!(self_admin_result.is_err()); + + let self_vault_result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + env.register_at( + &self_as_vault, + SorobanVaultGovernanceContract, + (&admin, &self_as_vault, &(0u64)), + ); + })); + assert!(self_vault_result.is_err()); +} + #[test] fn set_governance_rejects_obvious_invalid_contract_targets() { let env = Env::default(); From 4ba73c6bc942de26714cceda922ba70957d04901 Mon Sep 17 00:00:00 2001 From: carrion256 Date: Mon, 18 May 2026 17:39:29 +0200 Subject: [PATCH 4/4] fix: bind share token admin to vault #FIND-084 --- contract/vault/soroban/share-token/src/lib.rs | 22 ++++++++-- .../vault/soroban/share-token/src/tests.rs | 41 ++++++++++++++++++- 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/contract/vault/soroban/share-token/src/lib.rs b/contract/vault/soroban/share-token/src/lib.rs index 8ca152946..c30044d22 100644 --- a/contract/vault/soroban/share-token/src/lib.rs +++ b/contract/vault/soroban/share-token/src/lib.rs @@ -1,9 +1,14 @@ #![no_std] +#[cfg(test)] +extern crate std; + mod types; pub use types::*; -use soroban_sdk::{contract, contractimpl, panic_with_error, Address, Env, MuxedAddress, String}; +use soroban_sdk::{ + contract, contractimpl, panic_with_error, Address, Env, Executable, MuxedAddress, String, +}; use stellar_tokens::fungible::{ burnable::{emit_burn, FungibleBurnable}, Base, FungibleToken, @@ -95,6 +100,7 @@ impl SorobanShareTokenContract { ) { extend_instance_ttl(&env); require_contract_address(&env, &vault); + require_vault_admin(&env, &admin, &vault); env.storage().instance().set(&DataKey::Admin, &admin); env.storage().instance().set(&DataKey::Vault, &vault); Base::set_metadata(&env, decimals, name, symbol); @@ -109,6 +115,8 @@ impl SorobanShareTokenContract { pub fn set_admin(env: Env, caller: Address, admin: Address) { extend_instance_ttl(&env); require_admin(&env, &caller); + let vault = Self::vault(env.clone()); + require_vault_admin(&env, &admin, &vault); env.storage().instance().set(&DataKey::Admin, &admin); } @@ -171,8 +179,10 @@ fn require_vault_invoker(env: &Env) { } fn is_contract_address(addr: &Address) -> bool { - let bytes = addr.to_string().to_bytes(); - matches!(bytes.get(0), Some(b'C')) + matches!( + addr.executable(), + Some(Executable::Wasm(_)) | Some(Executable::StellarAsset) + ) } fn require_contract_address(env: &Env, addr: &Address) { @@ -181,6 +191,12 @@ fn require_contract_address(env: &Env, addr: &Address) { } } +fn require_vault_admin(env: &Env, admin: &Address, vault: &Address) { + if admin != vault { + panic_with_error!(env, ShareTokenError::InvalidInput); + } +} + fn extend_instance_ttl(env: &Env) { env.storage() .instance() diff --git a/contract/vault/soroban/share-token/src/tests.rs b/contract/vault/soroban/share-token/src/tests.rs index 82645bcca..e43cdb319 100644 --- a/contract/vault/soroban/share-token/src/tests.rs +++ b/contract/vault/soroban/share-token/src/tests.rs @@ -35,8 +35,8 @@ fn setup() -> (Env, Address, Address, Address) { ..Default::default() }); - let admin = Address::generate(&env); let vault = env.register(VaultCaller, ()); + let admin = vault.clone(); let token = env.register( SorobanShareTokenContract, ( @@ -50,6 +50,45 @@ fn setup() -> (Env, Address, Address, Address) { (env, admin, vault, token) } +#[test] +fn constructor_rejects_external_share_token_admin() { + let env = Env::default(); + env.mock_all_auths(); + let external_admin = Address::generate(&env); + let vault = env.register(VaultCaller, ()); + let token = Address::generate(&env); + + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + env.register_at( + &token, + SorobanShareTokenContract, + ( + &external_admin, + &vault, + &String::from_str(&env, "Templar Share"), + &String::from_str(&env, "tvSHARE"), + &7u32, + ), + ); + })); + + assert!(result.is_err()); +} + +#[test] +fn set_admin_rejects_non_vault_admin() { + let (env, _admin, vault, _token) = setup(); + let new_admin = Address::generate(&env); + + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + env.as_contract(&vault, || { + SorobanShareTokenContract::set_admin(env.clone(), vault.clone(), new_admin.clone()); + }); + })); + + assert!(result.is_err()); +} + #[test] fn vault_can_mint() { let (env, _admin, vault, token) = setup();