diff --git a/contracts/attestation_engine/src/lib.rs b/contracts/attestation_engine/src/lib.rs index b66f822..d03f90a 100644 --- a/contracts/attestation_engine/src/lib.rs +++ b/contracts/attestation_engine/src/lib.rs @@ -198,11 +198,22 @@ impl AttestationEngineContract { // Verifier Whitelist Management // ======================================================================== - /// Add a verifier to the whitelist + /// Add a verifier to the allowlist. /// /// # Arguments /// * `caller` - Must be admin /// * `verifier` - Address to add as authorized verifier + /// + /// # Errors + /// * `NotInitialized` – contract not initialized + /// * `Unauthorized` – caller is not admin + /// + /// # Security Notes + /// - Rate-limited per caller via `RateLimiter` (configurable via `set_rate_limit` + /// using function symbol `"add_verif"`). + /// - Duplicate adds are idempotent: emits `VerifAddAbuse` audit event and returns + /// `Ok(())` without modifying state, so call patterns are visible on-chain + /// without disrupting operation. pub fn add_verifier( e: Env, caller: Address, @@ -221,23 +232,53 @@ impl AttestationEngineContract { return Err(AttestationError::Unauthorized); } - // Add verifier to whitelist + // Rate-limit allowlist mutations per caller (panics if limit exceeded) + let fn_symbol = Symbol::new(&e, "add_verif"); + RateLimiter::check(&e, &caller, &fn_symbol); + + // Abuse case: duplicate add — emit audit event and return idempotently + let already_listed: bool = e + .storage() + .instance() + .get(&DataKey::Verifier(verifier.clone())) + .unwrap_or(false); + if already_listed { + e.events().publish( + (Symbol::new(&e, "VerifAddAbuse"),), + (caller, verifier, e.ledger().timestamp()), + ); + return Ok(()); + } + + // Add verifier to allowlist e.storage() .instance() .set(&DataKey::Verifier(verifier.clone()), &true); - // Emit event - e.events() - .publish((Symbol::new(&e, "VerifierAdded"),), (verifier,)); + // Emit audit event with caller and timestamp + e.events().publish( + (Symbol::new(&e, "VerifierAdded"),), + (caller, verifier, e.ledger().timestamp()), + ); Ok(()) } - /// Remove a verifier from the whitelist + /// Remove a verifier from the allowlist. /// /// # Arguments /// * `caller` - Must be admin /// * `verifier` - Address to remove from authorized verifiers + /// + /// # Errors + /// * `NotInitialized` – contract not initialized + /// * `Unauthorized` – caller is not admin + /// + /// # Security Notes + /// - Rate-limited per caller via `RateLimiter` (configurable via `set_rate_limit` + /// using function symbol `"rm_verif"`). + /// - Removing an address not in the allowlist is idempotent: emits `VerifRmAbuse` + /// audit event and returns `Ok(())` without modifying state. pub fn remove_verifier( e: Env, caller: Address, @@ -256,14 +297,34 @@ impl AttestationEngineContract { return Err(AttestationError::Unauthorized); } - // Remove verifier from whitelist + // Rate-limit allowlist mutations per caller (panics if limit exceeded) + let fn_symbol = Symbol::new(&e, "rm_verif"); + RateLimiter::check(&e, &caller, &fn_symbol); + + // Abuse case: remove of non-existent verifier — emit audit event and return idempotently + let is_listed: bool = e + .storage() + .instance() + .get(&DataKey::Verifier(verifier.clone())) + .unwrap_or(false); + if !is_listed { + e.events().publish( + (Symbol::new(&e, "VerifRmAbuse"),), + (caller, verifier, e.ledger().timestamp()), + ); + return Ok(()); + } + + // Remove verifier from allowlist e.storage() .instance() .remove(&DataKey::Verifier(verifier.clone())); - // Emit event - e.events() - .publish((Symbol::new(&e, "VerifierRemoved"),), (verifier,)); + // Emit audit event with caller and timestamp + e.events().publish( + (Symbol::new(&e, "VerifierRemoved"),), + (caller, verifier, e.ledger().timestamp()), + ); Ok(()) } diff --git a/contracts/attestation_engine/src/tests.rs b/contracts/attestation_engine/src/tests.rs index 33e5930..023ec75 100644 --- a/contracts/attestation_engine/src/tests.rs +++ b/contracts/attestation_engine/src/tests.rs @@ -1,7 +1,7 @@ #![cfg(test)] use super::*; -use soroban_sdk::{testutils::Address as _, Address, Env, Map, String}; +use soroban_sdk::{testutils::Address as _, testutils::Ledger as _, Address, Env, Map, String, Symbol}; #[test] fn test_initialize_and_getters() { @@ -662,3 +662,239 @@ fn test_record_drawdown_exceeds_max_loss_records_violation() { assert_eq!(violation_type, String::from_str(&e, "max_loss_exceeded")); assert_eq!(severity, String::from_str(&e, "high")); } + +// ============================================ +// Verifier Allowlist Abuse Cases +// ============================================ + +#[test] +fn test_add_verifier_success() { + let e = Env::default(); + e.mock_all_auths(); + let contract_id = e.register_contract(None, AttestationEngineContract); + let admin = Address::generate(&e); + let core = Address::generate(&e); + let verifier = Address::generate(&e); + + e.as_contract(&contract_id, || { + AttestationEngineContract::initialize(e.clone(), admin.clone(), core.clone()).unwrap(); + }); + + let result = e.as_contract(&contract_id, || { + AttestationEngineContract::add_verifier(e.clone(), admin.clone(), verifier.clone()) + }); + assert_eq!(result, Ok(())); + + let is_listed = e.as_contract(&contract_id, || { + AttestationEngineContract::is_verifier(e.clone(), verifier.clone()) + }); + assert!(is_listed, "Verifier should be listed after add"); +} + +#[test] +fn test_add_verifier_duplicate_is_idempotent() { + let e = Env::default(); + e.mock_all_auths(); + let contract_id = e.register_contract(None, AttestationEngineContract); + let admin = Address::generate(&e); + let core = Address::generate(&e); + let verifier = Address::generate(&e); + + e.as_contract(&contract_id, || { + AttestationEngineContract::initialize(e.clone(), admin.clone(), core.clone()).unwrap(); + }); + + // First add — normal path + let r1 = e.as_contract(&contract_id, || { + AttestationEngineContract::add_verifier(e.clone(), admin.clone(), verifier.clone()) + }); + assert_eq!(r1, Ok(())); + + // Second add — abuse path: idempotent, emits VerifAddAbuse event + let r2 = e.as_contract(&contract_id, || { + AttestationEngineContract::add_verifier(e.clone(), admin.clone(), verifier.clone()) + }); + assert_eq!(r2, Ok(())); + + // Verifier must still be listed + let still_listed = e.as_contract(&contract_id, || { + AttestationEngineContract::is_verifier(e.clone(), verifier.clone()) + }); + assert!(still_listed, "Verifier should remain listed after duplicate add"); +} + +#[test] +fn test_add_verifier_unauthorized() { + let e = Env::default(); + e.mock_all_auths(); + let contract_id = e.register_contract(None, AttestationEngineContract); + let admin = Address::generate(&e); + let core = Address::generate(&e); + let non_admin = Address::generate(&e); + let verifier = Address::generate(&e); + + e.as_contract(&contract_id, || { + AttestationEngineContract::initialize(e.clone(), admin.clone(), core.clone()).unwrap(); + }); + + let result = e.as_contract(&contract_id, || { + AttestationEngineContract::add_verifier(e.clone(), non_admin.clone(), verifier.clone()) + }); + assert_eq!(result, Err(AttestationError::Unauthorized)); + + // Verifier must not have been added + let is_listed = e.as_contract(&contract_id, || { + AttestationEngineContract::is_verifier(e.clone(), verifier.clone()) + }); + assert!(!is_listed, "Verifier must not be listed after unauthorized add attempt"); +} + +#[test] +fn test_remove_verifier_success() { + let e = Env::default(); + e.mock_all_auths(); + let contract_id = e.register_contract(None, AttestationEngineContract); + let admin = Address::generate(&e); + let core = Address::generate(&e); + let verifier = Address::generate(&e); + + e.as_contract(&contract_id, || { + AttestationEngineContract::initialize(e.clone(), admin.clone(), core.clone()).unwrap(); + AttestationEngineContract::add_verifier(e.clone(), admin.clone(), verifier.clone()).unwrap(); + }); + + let result = e.as_contract(&contract_id, || { + AttestationEngineContract::remove_verifier(e.clone(), admin.clone(), verifier.clone()) + }); + assert_eq!(result, Ok(())); + + let is_listed = e.as_contract(&contract_id, || { + AttestationEngineContract::is_verifier(e.clone(), verifier.clone()) + }); + assert!(!is_listed, "Verifier should not be listed after remove"); +} + +#[test] +fn test_remove_verifier_not_listed_is_idempotent() { + let e = Env::default(); + e.mock_all_auths(); + let contract_id = e.register_contract(None, AttestationEngineContract); + let admin = Address::generate(&e); + let core = Address::generate(&e); + let verifier = Address::generate(&e); + + e.as_contract(&contract_id, || { + AttestationEngineContract::initialize(e.clone(), admin.clone(), core.clone()).unwrap(); + }); + + // verifier was never added; remove is idempotent, emits VerifRmAbuse event + let result = e.as_contract(&contract_id, || { + AttestationEngineContract::remove_verifier(e.clone(), admin.clone(), verifier.clone()) + }); + assert_eq!(result, Ok(())); + + let is_listed = e.as_contract(&contract_id, || { + AttestationEngineContract::is_verifier(e.clone(), verifier.clone()) + }); + assert!(!is_listed, "Verifier should remain unlisted after no-op remove"); +} + +#[test] +fn test_remove_verifier_unauthorized() { + let e = Env::default(); + e.mock_all_auths(); + let contract_id = e.register_contract(None, AttestationEngineContract); + let admin = Address::generate(&e); + let core = Address::generate(&e); + let non_admin = Address::generate(&e); + let verifier = Address::generate(&e); + + e.as_contract(&contract_id, || { + AttestationEngineContract::initialize(e.clone(), admin.clone(), core.clone()).unwrap(); + AttestationEngineContract::add_verifier(e.clone(), admin.clone(), verifier.clone()).unwrap(); + }); + + let result = e.as_contract(&contract_id, || { + AttestationEngineContract::remove_verifier(e.clone(), non_admin.clone(), verifier.clone()) + }); + assert_eq!(result, Err(AttestationError::Unauthorized)); + + // Verifier must still be listed + let still_listed = e.as_contract(&contract_id, || { + AttestationEngineContract::is_verifier(e.clone(), verifier.clone()) + }); + assert!(still_listed, "Verifier must remain listed after unauthorized remove attempt"); +} + +#[test] +#[should_panic(expected = "Rate limit exceeded")] +fn test_add_verifier_rate_limit_exceeded() { + let e = Env::default(); + e.mock_all_auths(); + e.ledger().with_mut(|l| l.timestamp = 1000); + let contract_id = e.register_contract(None, AttestationEngineContract); + let admin = Address::generate(&e); + let core = Address::generate(&e); + + e.as_contract(&contract_id, || { + AttestationEngineContract::initialize(e.clone(), admin.clone(), core.clone()).unwrap(); + // 1 add_verifier allowed per 3600-second window + AttestationEngineContract::set_rate_limit( + e.clone(), + admin.clone(), + Symbol::new(&e, "add_verif"), + 3600u64, + 1u32, + ) + .unwrap(); + }); + + let verifier1 = Address::generate(&e); + let verifier2 = Address::generate(&e); + + e.as_contract(&contract_id, || { + // First call — within limit + AttestationEngineContract::add_verifier(e.clone(), admin.clone(), verifier1.clone()) + .unwrap(); + // Second call — exceeds limit, must panic + AttestationEngineContract::add_verifier(e.clone(), admin.clone(), verifier2.clone()) + .unwrap(); + }); +} + +#[test] +#[should_panic(expected = "Rate limit exceeded")] +fn test_remove_verifier_rate_limit_exceeded() { + let e = Env::default(); + e.mock_all_auths(); + e.ledger().with_mut(|l| l.timestamp = 1000); + let contract_id = e.register_contract(None, AttestationEngineContract); + let admin = Address::generate(&e); + let core = Address::generate(&e); + let verifier1 = Address::generate(&e); + let verifier2 = Address::generate(&e); + + e.as_contract(&contract_id, || { + AttestationEngineContract::initialize(e.clone(), admin.clone(), core.clone()).unwrap(); + AttestationEngineContract::add_verifier(e.clone(), admin.clone(), verifier1.clone()).unwrap(); + AttestationEngineContract::add_verifier(e.clone(), admin.clone(), verifier2.clone()).unwrap(); + // 1 remove_verifier allowed per 3600-second window + AttestationEngineContract::set_rate_limit( + e.clone(), + admin.clone(), + Symbol::new(&e, "rm_verif"), + 3600u64, + 1u32, + ) + .unwrap(); + }); + + e.as_contract(&contract_id, || { + // First remove — within limit + AttestationEngineContract::remove_verifier(e.clone(), admin.clone(), verifier1.clone()) + .unwrap(); + // Second remove — exceeds limit, must panic + AttestationEngineContract::remove_verifier(e.clone(), admin.clone(), verifier2.clone()) + .unwrap(); + }); +} diff --git a/contracts/shared_utils/src/lib.rs b/contracts/shared_utils/src/lib.rs index b3deb80..a2f802e 100644 --- a/contracts/shared_utils/src/lib.rs +++ b/contracts/shared_utils/src/lib.rs @@ -30,23 +30,14 @@ pub mod validation; #[cfg(test)] mod tests; -// Re-export commonly used items (explicit only to avoid E0252 glob clashes) +// Re-export all public items from each utility module pub use access_control::AccessControl; pub use batch::{ BatchConfig, BatchDataKey, BatchError, BatchMode, BatchOperationReport, BatchProcessor, BatchResultString, BatchResultVoid, DetailedBatchError, RollbackHelper, StateSnapshot, }; pub use emergency::EmergencyControl; -pub use error_codes::{category, code, emit_error_event, message_for_code}; -pub use errors::ErrorHelper; -pub use events::Events; pub use fees; -pub use math::SafeMath; -pub use pausable::Pausable; -pub use rate_limiting::RateLimiter; -pub use storage::Storage; -pub use time::TimeUtils; -pub use validation::Validation; pub use error_codes::*; pub use errors::*; pub use events::*; diff --git a/contracts/shared_utils/src/pausable.rs b/contracts/shared_utils/src/pausable.rs index dbe35c7..c074bb5 100644 --- a/contracts/shared_utils/src/pausable.rs +++ b/contracts/shared_utils/src/pausable.rs @@ -23,7 +23,6 @@ impl Pausable { /// # Returns /// `true` if paused, `false` otherwise pub fn is_paused(e: &Env) -> bool { - let paused_key = symbol_short!("paused"); e.storage() .instance() .get::<_, bool>(&Self::paused_key(e))