From 9200a2c52b67f12db31a44d9ff68c29f2e629a9c Mon Sep 17 00:00:00 2001 From: supreme2580 Date: Sat, 27 Jun 2026 16:32:21 +0100 Subject: [PATCH] feat(registry): check for zero address validation in registry set (closes #581) --- contracts/registry/src/lib.rs | 64 +++++++++++++++++++++++------------ 1 file changed, 43 insertions(+), 21 deletions(-) diff --git a/contracts/registry/src/lib.rs b/contracts/registry/src/lib.rs index 79d68e56..61947211 100644 --- a/contracts/registry/src/lib.rs +++ b/contracts/registry/src/lib.rs @@ -5,7 +5,7 @@ //! allowing for easy discovery and upgrades across the protocol. use soroban_sdk::{ - contract, contractimpl, contracttype, symbol_short, Address, Env, String, Vec, + contract, contractimpl, contracttype, symbol_short, xdr::ToXdr, Address, Bytes, Env, String, Vec, }; /// Contract metadata stored in the registry @@ -80,6 +80,8 @@ impl Registry { let stored_admin: Address = env.storage().instance().get(&DataKey::Admin).expect("admin not set"); assert!(admin == stored_admin, "unauthorized"); + Self::check_not_zero_address(&env, &address); + let contract_key = DataKey::Contract(contract_type.clone()); let timestamp = env.ledger().timestamp(); @@ -372,6 +374,17 @@ impl Registry { fn check_not_paused(env: &Env) { assert!(!Self::is_paused(env.clone()), "registry is paused"); } + + fn check_not_zero_address(env: &Env, address: &Address) { + let xdr: Bytes = address.clone().to_xdr(env); + let start = if xdr.len() > 32 { xdr.len() - 32 } else { 0 }; + for i in start..xdr.len() { + if xdr.get(i).unwrap() != 0 { + return; + } + } + panic!("cannot register zero address"); + } } #[cfg(test)] @@ -411,10 +424,10 @@ mod tests { client.register_contract(&admin, &contract_type, &address, &version); - assert!(client.is_registered(contract_type.clone())); - assert_eq!(client.get_address(contract_type.clone()), address); - assert_eq!(client.get_version(contract_type.clone()), version); - assert!(client.is_active(contract_type.clone())); + assert!(client.is_registered(&contract_type)); + assert_eq!(client.get_address(&contract_type), address); + assert_eq!(client.get_version(&contract_type), version); + assert!(client.is_active(&contract_type)); } #[test] @@ -433,7 +446,7 @@ mod tests { client.register_contract(&admin, &contract_type, &address2, &version2); - let info = client.get_contract(contract_type.clone()); + let info = client.get_contract(&contract_type); assert_eq!(info.address, address2); assert_eq!(info.version, version2); assert_eq!(info.previous_version, Some(address1)); @@ -448,10 +461,10 @@ mod tests { let version = String::from_str(&env, "1.0.0"); client.register_contract(&admin, &contract_type, &address, &version); - assert!(client.is_active(contract_type.clone())); + assert!(client.is_active(&contract_type)); client.deactivate_contract(&admin, &contract_type); - assert!(!client.is_active(contract_type.clone())); + assert!(!client.is_active(&contract_type)); } #[test] @@ -466,7 +479,7 @@ mod tests { client.deactivate_contract(&admin, &contract_type); client.activate_contract(&admin, &contract_type); - assert!(client.is_active(contract_type.clone())); + assert!(client.is_active(&contract_type)); } #[test] @@ -478,10 +491,10 @@ mod tests { let version = String::from_str(&env, "1.0.0"); client.register_contract(&admin, &contract_type, &address, &version); - assert!(client.is_registered(contract_type.clone())); + assert!(client.is_registered(&contract_type)); client.remove_contract(&admin, &contract_type); - assert!(!client.is_registered(contract_type.clone())); + assert!(!client.is_registered(&contract_type)); } #[test] @@ -565,11 +578,26 @@ mod tests { client.register_contract(&admin, &contract_type, &v1_address, &String::from_str(&env, "1.0.0")); client.register_contract(&admin, &contract_type, &v2_address, &String::from_str(&env, "2.0.0")); - let history = client.get_upgrade_history(contract_type); + let history = client.get_upgrade_history(&contract_type); assert_eq!(history.len(), 1); assert_eq!(history.get(0).unwrap(), v2_address); } + #[test] + #[should_panic(expected = "cannot register zero address")] + fn test_register_with_zero_address() { + let (env, client, admin) = setup(); + + use soroban_sdk::xdr::{Hash, ScAddress}; + use soroban_sdk::TryFromVal; + + let contract_type = String::from_str(&env, "AMM"); + let zero_address = Address::try_from_val(&env, &ScAddress::Contract(Hash([0u8; 32]))).unwrap(); + let version = String::from_str(&env, "1.0.0"); + + client.register_contract(&admin, &contract_type, &zero_address, &version); + } + #[test] #[should_panic(expected = "unauthorized")] fn test_unauthorized_register() { @@ -585,20 +613,14 @@ mod tests { fn test_multiple_contracts() { let (env, client, admin) = setup(); - let contracts = vec![ - ("AMM", "1.0.0"), - ("Lending", "1.0.0"), - ("Bridge", "1.0.0"), - ("XLMWrapper", "1.0.0"), - ("LiquidStaking", "1.0.0"), - ]; + let contract_names = ["AMM", "Lending", "Bridge", "XLMWrapper", "LiquidStaking"]; - for (name, version) in contracts.iter() { + for name in contract_names.iter() { client.register_contract( &admin, &String::from_str(&env, name), &Address::generate(&env), - &String::from_str(&env, version), + &String::from_str(&env, "1.0.0"), ); }