chore(contracts): validate payments vector length to prevent resource exhaustion (#252)

INVARIANTS.md

@@ -447,6 +447,8 @@ Pure accessors such as [`get_admin`](contracts/settlement/src/lib.rs#L140), [`ge
 - `distribute` / `batch_distribute` also require:
   - Positive amount(s)
   - Sufficient on-contract USDC balance before transfer
+- `batch_distribute` additionally requires:
+  - `1 <= payments.len() <= MAX_BATCH_SIZE` (50)
 
 **Post-conditions**
 - No address other than the current revenue-pool admin can emit administrative payment events or move USDC out of the revenue pool.

SECURITY.md

@@ -145,6 +145,9 @@ The Revenue Pool contract (`contracts/revenue_pool`) operates under the followin
 - **Operational Griefing (Balances):** Anyone can effectively transfer USDC to the revenue pool. If an attacker sends unsolicited funds, it increases the `balance()` but does not disrupt the `distribute` logic, as distribution is explicitly controlled by the admin.
   - *Mitigation:* The pool does not rely on strict balance equality invariants for its core operations, mitigating balance-based operational griefing. The `receive_payment` entrypoint is admin-only and event-only (no token movement), so indexers should reconcile `receive_payment` logs with actual token transfers.
 
+- **Resource Exhaustion via Unbounded Batch:** `batch_distribute` accepts a `Vec<(Address, i128)>`. Without a cap, a compromised admin key could submit thousands of entries, exhausting Soroban's per-transaction CPU/memory budget and causing unpredictable mid-execution failures.
+  - *Mitigation:* `batch_distribute` enforces `1 <= payments.len() <= MAX_BATCH_SIZE` (currently **50**), matching the vault's `batch_deduct` cap. Empty vectors and oversized vectors are rejected before any iteration or USDC transfer occurs. The cap keeps resource consumption well within Soroban network limits.
+
 ### Input Validation
 
 - [ ] All amounts validated to be > 0

contracts/revenue_pool/src/lib.rs

@@ -21,6 +21,11 @@ const ERR_UNAUTHORIZED: &str = "unauthorized: caller is not admin";
 const ERR_INSUFFICIENT_BALANCE: &str = "insufficient USDC balance";
 const ERR_NOT_INITIALIZED: &str = "revenue pool not initialized";
 
+/// Maximum number of payments allowed in a single `batch_distribute` call.
+/// Caps CPU/memory usage well within Soroban resource limits and aligns with
+/// the vault's `MAX_BATCH_SIZE` for `batch_deduct`.
+pub const MAX_BATCH_SIZE: u32 = 50;
+
 #[contract]
 pub struct RevenuePool;
 
@@ -255,8 +260,11 @@ impl RevenuePool {
     /// * `env` - The environment running the contract.
     /// * `caller` - Must be the current admin.
     /// * `payments` - A vector of `(Address, i128)` tuples representing destinations and amounts.
+    ///   Must contain between 1 and [`MAX_BATCH_SIZE`] entries (inclusive).
     ///
     /// # Panics
+    /// * If `payments` is empty (`"batch_distribute requires at least one payment"`).
+    /// * If `payments` exceeds [`MAX_BATCH_SIZE`] entries (`"batch too large"`).
     /// * If the caller is not the current admin (`"unauthorized: caller is not admin"`).
     /// * If any individual amount is zero or negative (`"amount must be positive"`).
     /// * If the revenue pool has not been initialized.
@@ -271,6 +279,14 @@ impl RevenuePool {
             panic!("{}", ERR_UNAUTHORIZED);
         }
 
+        let n = payments.len();
+        if n == 0 {
+            panic!("batch_distribute requires at least one payment");
+        }
+        if n > MAX_BATCH_SIZE {
+            panic!("batch too large");
+        }
+
         let mut total_amount: i128 = 0;
         for payment in payments.iter() {
             let (_, amount) = payment;

contracts/revenue_pool/src/test.rs

@@ -761,3 +761,119 @@ fn get_usdc_token_before_init_panics() {
 
     client.get_usdc_token();
 }
+
+// ---------------------------------------------------------------------------
+// batch_distribute length-cap tests (resource exhaustion prevention)
+// ---------------------------------------------------------------------------
+
+#[test]
+#[should_panic(expected = "batch_distribute requires at least one payment")]
+fn batch_distribute_empty_panics() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let admin = Address::generate(&env);
+    let (_, client) = create_pool(&env);
+    let (usdc_address, _, _) = create_usdc(&env, &admin);
+
+    client.init(&admin, &usdc_address);
+
+    let payments: Vec<(Address, i128)> = Vec::new(&env);
+    client.batch_distribute(&admin, &payments);
+}
+
+#[test]
+#[should_panic(expected = "batch too large")]
+fn batch_distribute_too_large_panics() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let admin = Address::generate(&env);
+    let (pool_addr, client) = create_pool(&env);
+    let (usdc_address, _, usdc_admin) = create_usdc(&env, &admin);
+
+    client.init(&admin, &usdc_address);
+    fund_pool(&usdc_admin, &pool_addr, 100_000);
+
+    // Build a batch of MAX_BATCH_SIZE + 1 entries
+    let mut payments: Vec<(Address, i128)> = Vec::new(&env);
+    for _ in 0..=crate::MAX_BATCH_SIZE {
+        payments.push_back((Address::generate(&env), 1_i128));
+    }
+    client.batch_distribute(&admin, &payments);
+}
+
+#[test]
+fn batch_distribute_at_max_size_succeeds() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let admin = Address::generate(&env);
+    let (pool_addr, client) = create_pool(&env);
+    let (usdc_address, usdc_client, usdc_admin) = create_usdc(&env, &admin);
+
+    client.init(&admin, &usdc_address);
+    let amount_per = 10_i128;
+    let total = amount_per * (crate::MAX_BATCH_SIZE as i128);
+    fund_pool(&usdc_admin, &pool_addr, total);
+
+    // Build a batch of exactly MAX_BATCH_SIZE entries
+    let mut payments: Vec<(Address, i128)> = Vec::new(&env);
+    for _ in 0..crate::MAX_BATCH_SIZE {
+        payments.push_back((Address::generate(&env), amount_per));
+    }
+    client.batch_distribute(&admin, &payments);
+
+    // Pool should be drained
+    assert_eq!(usdc_client.balance(&pool_addr), 0);
+}
+
+#[test]
+#[should_panic(expected = "amount must be positive")]
+fn batch_distribute_negative_amount_panics() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let admin = Address::generate(&env);
+    let dev = Address::generate(&env);
+    let (_, client) = create_pool(&env);
+    let (usdc_address, _, _) = create_usdc(&env, &admin);
+
+    client.init(&admin, &usdc_address);
+
+    let mut payments: Vec<(Address, i128)> = Vec::new(&env);
+    payments.push_back((dev, -100));
+    client.batch_distribute(&admin, &payments);
+}
+
+#[test]
+#[should_panic(expected = "unauthorized: caller is not admin")]
+fn batch_distribute_unauthorized_panics() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let admin = Address::generate(&env);
+    let attacker = Address::generate(&env);
+    let dev = Address::generate(&env);
+    let (pool_addr, client) = create_pool(&env);
+    let (usdc_address, _, usdc_admin) = create_usdc(&env, &admin);
+
+    client.init(&admin, &usdc_address);
+    fund_pool(&usdc_admin, &pool_addr, 1000);
+
+    let mut payments: Vec<(Address, i128)> = Vec::new(&env);
+    payments.push_back((dev, 100));
+    client.batch_distribute(&attacker, &payments);
+}
+
+#[test]
+#[should_panic(expected = "invalid recipient: cannot distribute to the contract itself")]
+fn batch_distribute_self_recipient_panics() {
+    let env = Env::default();
+    env.mock_all_auths();
+    let admin = Address::generate(&env);
+    let (pool_addr, client) = create_pool(&env);
+    let (usdc_address, _, usdc_admin) = create_usdc(&env, &admin);
+
+    client.init(&admin, &usdc_address);
+    fund_pool(&usdc_admin, &pool_addr, 1000);
+
+    let mut payments: Vec<(Address, i128)> = Vec::new(&env);
+    payments.push_back((pool_addr, 100));
+    client.batch_distribute(&admin, &payments);
+}

contracts/settlement/src/test.rs

@@ -16,7 +16,7 @@ mod settlement_tests {
         let addr = env.register(CalloraSettlement, ());
         let client = CalloraSettlementClient::new(&env, &addr);
         client.init(&admin, &vault);
-        let _third_party = Address::generate(&env);
+        let third_party = Address::generate(&env);
         (env, addr, admin, vault, third_party)
     }
 
@@ -1036,7 +1036,7 @@ mod settlement_tests {
 
     #[test]
     fn test_accept_admin_authorization_matrix() {
-        let (env, addr, admin, vault, third_party) = setup_contract();
+        let (env, addr, admin, _vault, _third_party) = setup_contract();
         let client = CalloraSettlementClient::new(&env, &addr);
         let new_admin = Address::generate(&env);
 
@@ -1047,8 +1047,6 @@ mod settlement_tests {
         assert_eq!(client.get_admin(), new_admin);
     }
 
-
-
     #[test]
     fn test_get_all_developer_balances_authorization_matrix() {
         let (env, addr, admin, vault, third_party) = setup_contract();

contracts/vault/src/lib.rs

@@ -59,7 +59,7 @@ impl CalloraVault {
     ) -> VaultMeta {
         owner.require_auth();
         let inst = env.storage().instance();
-        if inst.has(&StorageKey::MetaKey) {
+        if inst.has(&StorageKey::Meta) {
             panic!("vault already initialized");
         }
         assert!(
@@ -93,7 +93,7 @@ impl CalloraVault {
             authorized_caller,
             min_deposit: min_d,
         };
-        inst.set(&StorageKey::MetaKey, &meta);
+        inst.set(&StorageKey::Meta, &meta);
         inst.set(&StorageKey::UsdcToken, &usdc_token);
         inst.set(&StorageKey::Admin, &owner);
         if let Some(p) = revenue_pool {
@@ -118,6 +118,7 @@ impl CalloraVault {
         list.contains(&caller)
     }
 
+    #[allow(dead_code)]
     fn migrate(env: &Env) {
         let inst = env.storage().instance();
 
@@ -246,7 +247,7 @@ impl CalloraVault {
         let mut meta = Self::get_meta(env.clone());
         meta.owner.require_auth();
         meta.authorized_caller = Some(caller.clone());
-        env.storage().instance().set(&StorageKey::MetaKey, &meta);
+        env.storage().instance().set(&StorageKey::Meta, &meta);
         env.events().publish(
             (Symbol::new(&env, "set_auth_caller"), meta.owner.clone()),
             caller,
@@ -356,9 +357,14 @@ impl CalloraVault {
     }
 
     pub fn deduct(env: Env, caller: Address, amount: i128, request_id: Option<Symbol>) -> i128 {
+        Self::require_not_paused(env.clone());
         caller.require_auth();
         assert!(amount > 0, "amount must be positive");
-        let max_d = Self::get_max_deduct(env.clone());
+        let max_d: i128 = env
+            .storage()
+            .instance()
+            .get(&StorageKey::MaxDeduct)
+            .unwrap_or(DEFAULT_MAX_DEDUCT);
         assert!(amount <= max_d, "deduct amount exceeds max_deduct");
         let meta = Self::get_meta(env.clone());
         let auth = match &meta.authorized_caller {
@@ -368,7 +374,10 @@ impl CalloraVault {
         assert!(auth, "unauthorized caller");
         assert!(meta.balance >= amount, "insufficient balance");
         let mut meta = Self::get_meta(env.clone());
-        meta.balance = meta.balance.checked_sub(amount).unwrap_or_else(|| panic!("balance underflow"));
+        meta.balance = meta
+            .balance
+            .checked_sub(amount)
+            .unwrap_or_else(|| panic!("balance underflow"));
         env.storage().instance().set(&StorageKey::Meta, &meta);
         let inst = env.storage().instance();
         if let Some(s) = inst.get(&StorageKey::Settlement) {
@@ -395,7 +404,11 @@ impl CalloraVault {
         let n = items.len();
         assert!(n > 0, "batch_deduct requires at least one item");
         assert!(n <= MAX_BATCH_SIZE, "batch too large");
-        let max_d = Self::get_max_deduct(env.clone());
+        let max_d: i128 = env
+            .storage()
+            .instance()
+            .get(&StorageKey::MaxDeduct)
+            .unwrap_or(DEFAULT_MAX_DEDUCT);
         let mut meta = Self::get_meta(env.clone());
         let auth = match &meta.authorized_caller {
             Some(ac) => caller == *ac || caller == meta.owner,
@@ -408,8 +421,12 @@ impl CalloraVault {
             assert!(item.amount > 0, "amount must be positive");
             assert!(item.amount <= max_d, "deduct amount exceeds max_deduct");
             assert!(running >= item.amount, "insufficient balance");
-            running = running.checked_sub(item.amount).unwrap_or_else(|| panic!("balance underflow"));
-            total = total.checked_add(item.amount).unwrap_or_else(|| panic!("total overflow"));
+            running = running
+                .checked_sub(item.amount)
+                .unwrap_or_else(|| panic!("balance underflow"));
+            total = total
+                .checked_add(item.amount)
+                .unwrap_or_else(|| panic!("total overflow"));
         }
 
         let mut eb = meta.balance;
@@ -434,7 +451,7 @@ impl CalloraVault {
         }
 
         meta.balance = running;
-        env.storage().instance().set(&StorageKey::MetaKey, &meta);
+        env.storage().instance().set(&StorageKey::Meta, &meta);
         meta.balance
     }
 
@@ -472,7 +489,7 @@ impl CalloraVault {
         let mut meta = Self::get_meta(env.clone());
         let old = meta.owner.clone();
         meta.owner = pending;
-        env.storage().instance().set(&StorageKey::MetaKey, &meta);
+        env.storage().instance().set(&StorageKey::Meta, &meta);
         env.storage().instance().remove(&StorageKey::PendingOwner);
         env.events().publish(
             (Symbol::new(&env, "ownership_accepted"), old, meta.owner),
@@ -492,7 +509,10 @@ impl CalloraVault {
             .expect("vault not initialized");
         let usdc = token::Client::new(&env, &ua);
         usdc.transfer(&env.current_contract_address(), &meta.owner, &amount);
-        meta.balance = meta.balance.checked_sub(amount).unwrap_or_else(|| panic!("balance underflow"));
+        meta.balance = meta
+            .balance
+            .checked_sub(amount)
+            .unwrap_or_else(|| panic!("balance underflow"));
         env.storage().instance().set(&StorageKey::Meta, &meta);
         env.events().publish(
             (Symbol::new(&env, "withdraw"), meta.owner.clone()),
@@ -513,7 +533,10 @@ impl CalloraVault {
             .expect("vault not initialized");
         let usdc = token::Client::new(&env, &ua);
         usdc.transfer(&env.current_contract_address(), &to, &amount);
-        meta.balance = meta.balance.checked_sub(amount).unwrap_or_else(|| panic!("balance underflow"));
+        meta.balance = meta
+            .balance
+            .checked_sub(amount)
+            .unwrap_or_else(|| panic!("balance underflow"));
         env.storage().instance().set(&StorageKey::Meta, &meta);
         env.events().publish(
             (Symbol::new(&env, "withdraw_to"), meta.owner.clone(), to),