fix(security): require admin auth for MintToken in batch operations (#39)

peerx-contracts/counter/batch.rs

@@ -137,6 +137,8 @@ fn is_valid_token(token: &Symbol) -> bool {
 }
 
 fn authorize_batch_access(env: &Env, operations: &Vec<BatchOperation>) -> Result<(), Symbol> {
+    let mut requires_admin = false;
+
     for i in 0..operations.len() {
         if let Some(operation) = operations.get(i) {
             match operation {
@@ -147,11 +149,43 @@ fn authorize_batch_access(env: &Env, operations: &Vec<BatchOperation>) -> Result
                     crate::require_verified_user(env, &user)
                         .map_err(|_| Symbol::new(env, "kyc_req"))?;
                 }
-                BatchOperation::MintToken(_, _, _) => {}
+                // `MintToken` is privileged. Defer the admin gate until after the
+                // loop so it is enforced exactly once for the whole batch, no
+                // matter how many mint operations it contains.
+                BatchOperation::MintToken(_, _, _) => {
+                    requires_admin = true;
+                }
             }
         }
     }
 
+    // `MintToken` is an admin-only operation, even inside a batch. If any
+    // operation mints, the entire batch must be authorized by the configured
+    // contract admin. This closes the bypass (issue #39) where a batch whose
+    // first/only operation was a `MintToken` set `caller = None` and skipped
+    // every KYC and admin guard, letting anyone mint tokens.
+    if requires_admin {
+        require_batch_admin(env)?;
+    }
+
+    Ok(())
+}
+
+/// Requires that the current batch invocation is authorized by the configured
+/// contract admin. Used to gate privileged batch operations such as
+/// `MintToken`.
+///
+/// Returns `admin_req` when no admin is configured (fail closed) or when the
+/// loaded admin does not pass the shared admin check. Callers who are not the
+/// admin cannot satisfy `admin.require_auth()` and therefore revert.
+fn require_batch_admin(env: &Env) -> Result<(), Symbol> {
+    let admin = env
+        .storage()
+        .persistent()
+        .get::<_, Address>(&crate::storage::ADMIN_KEY)
+        .ok_or_else(|| Symbol::new(env, "admin_req"))?;
+    admin.require_auth();
+    crate::admin::require_admin(env, &admin).map_err(|_| Symbol::new(env, "admin_req"))?;
     Ok(())
 }
 

peerx-contracts/counter/src/kyc_tests.rs

@@ -505,3 +505,108 @@ fn test_sensitive_contract_entry_points_require_verified_kyc() {
     });
     assert_eq!(stake_id, 0);
 }
+
+// ===== Batch `MintToken` admin-gate regression tests (issue #39) =====
+//
+// Before the fix, `authorize_batch_access` did nothing for `MintToken` and the
+// `lib.rs` batch entry points set `caller = None` for a mint-first batch, so a
+// batch whose (first) operation was a `MintToken` skipped every KYC and admin
+// guard. Anyone could mint tokens by submitting such a batch. These tests pin
+// down that a batch containing a `MintToken` is now admin-only.
+//
+// NB: these tests intentionally do NOT mock auths, so that `admin.require_auth()`
+// is actually enforced and an unauthorized (non-admin) batch reverts.
+
+/// Sets the contract admin directly in persistent storage, matching how the
+/// production `set_admin` entry point persists it (`ADMIN_KEY`).
+fn set_stored_admin(env: &Env, contract_id: &Address, admin: &Address) {
+    with_contract(env, contract_id, || {
+        env.storage()
+            .persistent()
+            .set(&crate::storage::ADMIN_KEY, admin);
+    });
+}
+
+/// Builds a batch whose only operation mints `amount` of XLM to `to`.
+fn mint_only_batch(env: &Env, to: &Address, amount: i128) -> Vec<BatchOperation> {
+    let mut operations = Vec::new(env);
+    operations.push_back(BatchOperation::MintToken(
+        symbol_short!("XLM"),
+        to.clone(),
+        amount,
+    ));
+    operations
+}
+
+#[test]
+fn test_batch_mint_by_non_admin_reverts_atomic() {
+    let env = Env::default();
+    let contract_id = env.register(CounterContract, ());
+    let admin = Address::generate(&env);
+    let attacker = Address::generate(&env);
+
+    set_stored_admin(&env, &contract_id, &admin);
+
+    // A mint-only atomic batch submitted without the admin's authorization must
+    // revert (panic on the unmet `admin.require_auth()`), so `attacker` cannot
+    // mint tokens to itself.
+    let unauthorized_mint = panic::catch_unwind(AssertUnwindSafe(|| {
+        with_contract(&env, &contract_id, || {
+            let operations = mint_only_batch(&env, &attacker, 1_000_000);
+            CounterContract::execute_batch_atomic(env.clone(), operations)
+        })
+    }));
+
+    assert!(
+        unauthorized_mint.is_err(),
+        "non-admin atomic batch containing MintToken must revert"
+    );
+}
+
+#[test]
+fn test_batch_mint_by_non_admin_reverts_best_effort() {
+    let env = Env::default();
+    let contract_id = env.register(CounterContract, ());
+    let admin = Address::generate(&env);
+    let attacker = Address::generate(&env);
+
+    set_stored_admin(&env, &contract_id, &admin);
+
+    // The best-effort path shares `authorize_batch_access`, so it must also
+    // reject an unauthorized mint batch before any operation executes.
+    let unauthorized_mint = panic::catch_unwind(AssertUnwindSafe(|| {
+        with_contract(&env, &contract_id, || {
+            let operations = mint_only_batch(&env, &attacker, 1_000_000);
+            CounterContract::execute_batch_best_effort(env.clone(), operations)
+        })
+    }));
+
+    assert!(
+        unauthorized_mint.is_err(),
+        "non-admin best-effort batch containing MintToken must revert"
+    );
+}
+
+#[test]
+fn test_batch_mint_fails_closed_when_no_admin_configured() {
+    let env = Env::default();
+    let contract_id = env.register(CounterContract, ());
+    let attacker = Address::generate(&env);
+
+    // No admin is configured. The mint guard must fail closed: the batch is
+    // rejected rather than allowed to mint. The atomic entry point converts the
+    // authorization error into a failed (non-executed) result.
+    let result: BatchResult = with_contract(&env, &contract_id, || {
+        let operations = mint_only_batch(&env, &attacker, 1_000_000);
+        CounterContract::execute_batch_atomic(env.clone(), operations)
+    });
+
+    assert_eq!(
+        result.operations_executed, 0,
+        "no operation may execute when the mint guard cannot resolve an admin"
+    );
+    assert!(
+        result.operations_failed >= 1,
+        "mint batch must be reported as failed when no admin is configured"
+    );
+}

scripts/check_kyc_guards.sh

@@ -66,4 +66,27 @@ if [[ "$(grep -c 'authorize_batch_access(env, &operations)' "$BATCH_FILE")" -lt
   exit 1
 fi
 
+# `MintToken` is a privileged, admin-only batch operation (issue #39). The
+# shared authorization helper must gate any batch that mints behind the
+# contract admin, so a MintToken op can no longer bypass KYC/admin checks.
+if ! grep -q 'fn require_batch_admin' "$BATCH_FILE"; then
+  echo "Batch guard is missing the admin gate for privileged operations (MintToken)" >&2
+  exit 1
+fi
+
+if ! grep -q 'require_batch_admin(env)' "$BATCH_FILE"; then
+  echo "Batch authorization helper does not invoke the MintToken admin gate" >&2
+  exit 1
+fi
+
+if ! grep -q 'admin.require_auth();' "$BATCH_FILE"; then
+  echo "Batch admin gate is missing admin authentication enforcement" >&2
+  exit 1
+fi
+
+if ! grep -q 'crate::admin::require_admin(env, &admin)' "$BATCH_FILE"; then
+  echo "Batch admin gate is missing the shared require_admin check" >&2
+  exit 1
+fi
+
 echo "Verified KYC guard coverage for sensitive contract entry points."