fix: close protocol-pause bypass via admin-whitelist collision (#26)

contracts/utility_contracts/src/lib.rs

@@ -1454,11 +1454,7 @@ fn validate_reading(
 
     // Get last reading timestamp
     let last_reading_key = DataKey::LastReadingTime(meter_id);
-    let last_reading_time: u64 = env
-        .storage()
-        .instance()
-        .get(&last_reading_key)
-        .unwrap_or(0);
+    let last_reading_time: u64 = env.storage().instance().get(&last_reading_key).unwrap_or(0);
 
     // Check for duplicate or old timestamp
     if timestamp <= last_reading_time {
@@ -3800,6 +3796,11 @@ impl UtilityContract {
         per_stream_limit: i128,
         is_enabled: bool,
     ) {
+        // Verify admin param matches the registered AdminAddress
+        let stored_admin = get_admin_or_panic(&env);
+        if admin != stored_admin {
+            panic_with_error!(&env, ContractError::UnauthorizedAdmin);
+        }
         admin.require_auth();
 
         // Validate limits are positive
@@ -3880,6 +3881,11 @@ impl UtilityContract {
         expires_at: u64,
         reason: Symbol,
     ) {
+        // Verify admin param matches the registered AdminAddress
+        let stored_admin = get_admin_or_panic(&env);
+        if admin != stored_admin {
+            panic_with_error!(&env, ContractError::UnauthorizedAdmin);
+        }
         admin.require_auth();
 
         // Validate expiration time
@@ -3893,7 +3899,7 @@ impl UtilityContract {
             let _meter = get_meter_or_panic(&env, meter_id);
         }
 
-        apply_override(&env, admin, meter_id, expires_at, reason);
+        apply_override(&env, admin.clone(), meter_id, expires_at, reason.clone());
 
         // Emit audit event
         env.events().publish(
@@ -3935,15 +3941,16 @@ impl UtilityContract {
     /// UtilityContract::revoke_velocity_override(env, admin, meter_id);
     /// ```
     pub fn revoke_velocity_override(env: Env, admin: Address, meter_id: u64) {
+        // Verify admin param matches the registered AdminAddress
+        let stored_admin = get_admin_or_panic(&env);
+        if admin != stored_admin {
+            panic_with_error!(&env, ContractError::UnauthorizedAdmin);
+        }
         admin.require_auth();
 
         // Check if override exists before attempting to revoke
         // This prevents unnecessary storage operations and provides better error messages
-        let override_key = if meter_id == 0 {
-            DataKey::VelocityOverrideGlobal
-        } else {
-            DataKey::VelocityOverride(meter_id)
-        };
+        let override_key = velocity_limit::VelocityDataKey::VelocityOverride(meter_id);
 
         if !env.storage().instance().has(&override_key) {
             panic_with_error!(&env, ContractError::InvalidUsageValue);
@@ -4858,7 +4865,9 @@ impl UtilityContract {
             // Emit ReadingRejected event
             let reason = match e {
                 ContractError::InvalidReadingValue => String::from_str(&env, "Negative reading"),
-                ContractError::DuplicateTimestamp => String::from_str(&env, "Timestamp not after last"),
+                ContractError::DuplicateTimestamp => {
+                    String::from_str(&env, "Timestamp not after last")
+                }
                 ContractError::ReadingDeltaTooLarge => String::from_str(&env, "Reading too large"),
                 _ => String::from_str(&env, "Unknown validation error"),
             };
@@ -5073,9 +5082,10 @@ impl UtilityContract {
         update_provider_total_pool(&env, &meter.provider, old_meter_value, new_meter_value);
 
         // Update last reading time
-        env.storage()
-            .instance()
-            .set(&DataKey::LastReadingTime(signed_data.meter_id), &signed_data.timestamp);
+        env.storage().instance().set(
+            &DataKey::LastReadingTime(signed_data.meter_id),
+            &signed_data.timestamp,
+        );
 
         env.storage()
             .instance()

contracts/utility_contracts/src/test.rs

@@ -1012,3 +1012,89 @@ fn test_event_emissions() {
     // Note: In a real test environment, you would verify the events were emitted
     // This test ensures the functions execute without panicking when events are published
 }
+
+// ============================================================================
+// Issue #26 — Protocol-Pause Bypass via Admin-Whitelist Collision
+// ============================================================================
+
+#[test]
+fn test_configure_velocity_limits_rejects_non_admin() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let contract_id = env.register(UtilityContract, ());
+    let client = UtilityContractClient::new(&env, &contract_id);
+    let admin = Address::generate(&env);
+    client.set_admin(&admin);
+
+    let attacker = Address::generate(&env);
+    // attacker passes themselves as admin param — must be rejected
+    let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+        client.set_velocity_limit_config(&attacker, &1_000_000i128, &100_000i128, &true);
+    }));
+    assert!(result.is_err(), "non-admin should not be able to configure velocity limits");
+}
+
+#[test]
+fn test_configure_velocity_limits_succeeds_for_real_admin() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let contract_id = env.register(UtilityContract, ());
+    let client = UtilityContractClient::new(&env, &contract_id);
+    let admin = Address::generate(&env);
+    client.set_admin(&admin);
+
+    client.set_velocity_limit_config(&admin, &1_000_000i128, &100_000i128, &true);
+    let config = client.get_velocity_limits().unwrap();
+    assert_eq!(config.global_limit, 1_000_000);
+    assert_eq!(config.per_stream_limit, 100_000);
+    assert!(config.is_enabled);
+}
+
+#[test]
+fn test_apply_velocity_override_rejects_non_admin() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let contract_id = env.register(UtilityContract, ());
+    let client = UtilityContractClient::new(&env, &contract_id);
+    let admin = Address::generate(&env);
+    client.set_admin(&admin);
+
+    let attacker = Address::generate(&env);
+    let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+        client.apply_velocity_override(&attacker, &0u64, &0u64, &soroban_sdk::symbol_short!("test"));
+    }));
+    assert!(result.is_err(), "non-admin should not be able to apply velocity override");
+}
+
+#[test]
+fn test_apply_and_revoke_velocity_override_by_admin() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let contract_id = env.register(UtilityContract, ());
+    let client = UtilityContractClient::new(&env, &contract_id);
+    let admin = Address::generate(&env);
+    client.set_admin(&admin);
+
+    // Admin applies a global override (meter_id = 0), then revokes it
+    client.apply_velocity_override(&admin, &0u64, &0u64, &soroban_sdk::symbol_short!("maint"));
+    client.revoke_velocity_override(&admin, &0u64);
+}
+
+#[test]
+fn test_revoke_velocity_override_rejects_non_admin() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let contract_id = env.register(UtilityContract, ());
+    let client = UtilityContractClient::new(&env, &contract_id);
+    let admin = Address::generate(&env);
+    client.set_admin(&admin);
+
+    // Admin applies override first so it exists
+    client.apply_velocity_override(&admin, &0u64, &0u64, &soroban_sdk::symbol_short!("maint"));
+
+    let attacker = Address::generate(&env);
+    let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+        client.revoke_velocity_override(&attacker, &0u64);
+    }));
+    assert!(result.is_err(), "non-admin should not be able to revoke velocity override");
+}

contracts/utility_contracts/src/velocity_limit.rs

@@ -443,9 +443,8 @@ pub fn check_velocity_limits(
 ///
 /// `scope` = 0 for global override, or specific meter_id
 /// `expires_at` = 0 for permanent, or Unix timestamp for expiration
+/// NOTE: Caller (lib.rs) must have already verified admin auth against stored AdminAddress.
 pub fn apply_override(env: &Env, admin: Address, scope: u64, expires_at: u64, reason: Symbol) {
-    admin.require_auth();
-
     let now = env.ledger().timestamp();
 
     let override_config = VelocityOverride {
@@ -523,10 +522,8 @@ pub fn get_velocity_config(env: &Env) -> Option<VelocityConfig> {
         .get(&VelocityDataKey::VelocityConfig)
 }
 
-/// Update velocity configuration (admin only)
-pub fn set_velocity_config(env: &Env, admin: Address, config: VelocityConfig) {
-    admin.require_auth();
-
+/// Update velocity configuration (admin only - caller must have verified admin auth)
+pub fn set_velocity_config(env: &Env, _admin: Address, config: VelocityConfig) {
     env.storage()
         .instance()
         .set(&VelocityDataKey::VelocityConfig, &config);