fix(wrapper): require fee to cover forwarded cycles [stopgap]

src/wrapper/src/api/call.rs

@@ -16,16 +16,53 @@ pub async fn bridge_call(args: BridgeCallArgs) -> Result<Vec<u8>, String> {
         return Err("Self-calls are not allowed through the bridge.".to_string());
     }
 
-    // 1) Charge fee using the payment guard
     let p = args.payment.unwrap_or(PaymentType::AttachedCycles);
+    let cycles = args.cycles_to_forward.unwrap_or(0);
+
+    // 1) Validate that the forwarded cycles are covered by the fee *before*
+    //    charging anything.
+    //
+    //    The forwarded cycles are attached to the outbound call from this
+    //    canister's own balance. They are only replenished when the payment is
+    //    cycle-denominated: `AttachedCycles` (via `msg_cycles_accept`) and the
+    //    ICRC-2 *cycles* flows (via the cycles-ledger `withdraw_from(to: self)`)
+    //    credit this canister's cycle balance; the ICRC-2 *token* flows credit a
+    //    token account instead. Since `fee_amount` and `cycles_to_forward` are
+    //    both caller-controlled, we must ensure the fee is paid in cycles and
+    //    covers the amount forwarded -- otherwise a caller could pay a trivial
+    //    fee and drain the wrapper's cycles.
+    if cycles > 0 {
+        if !is_cycle_payment(&p) {
+            return Err(BridgeError::ForwardRequiresCyclePayment.to_string());
+        }
+        if args.fee_amount < cycles {
+            return Err(BridgeError::ForwardExceedsFee {
+                fee: args.fee_amount,
+                forward: cycles,
+            }
+            .to_string());
+        }
+    }
+
+    // 2) Charge fee using the payment guard
     PAYMENT_GUARD
         .deduct(p, args.fee_amount)
         .await
         .map_err(map_guard_err)?;
 
-    // 2) Forward the call to target
-    let cycles = args.cycles_to_forward.unwrap_or(0);
+    // 3) Forward the call to target
     forward_raw(args.target, &args.method, args.args, cycles)
         .await
         .map_err(|e| BridgeError::TargetRejected(e).to_string())
 }
+
+/// Whether a payment type credits this canister's *cycle* balance (as opposed to
+/// a token ledger account), and can therefore fund forwarded cycles.
+fn is_cycle_payment(payment: &PaymentType) -> bool {
+    matches!(
+        payment,
+        PaymentType::AttachedCycles
+            | PaymentType::CallerPaysIcrc2Cycles
+            | PaymentType::PatronPaysIcrc2Cycles(_)
+    )
+}

src/wrapper/src/domain/errors.rs

@@ -11,6 +11,13 @@ pub enum BridgeError {
     TargetRejected(String),
     /// Fee deduction failed (insufficient cycles/allowance/etc.).
     GuardError(String),
+    /// Cycles were requested to be forwarded, but the chosen payment type is not
+    /// cycle-denominated, so the forwarded cycles would come out of the wrapper's
+    /// own balance rather than being funded by the payment.
+    ForwardRequiresCyclePayment,
+    /// The requested cycles to forward exceed the fee charged, so the wrapper
+    /// would be forwarding more cycles than it was paid.
+    ForwardExceedsFee { fee: u128, forward: u128 },
 }
 
 impl fmt::Display for BridgeError {
@@ -19,6 +26,17 @@ impl fmt::Display for BridgeError {
             BridgeError::Candid(e) => write!(f, "Candid error: {e}"),
             BridgeError::TargetRejected(e) => write!(f, "Target canister rejected call: {e}"),
             BridgeError::GuardError(e) => write!(f, "Payment guard error: {e}"),
+            BridgeError::ForwardRequiresCyclePayment => write!(
+                f,
+                "Forwarding cycles requires a cycle-denominated payment type \
+                 (AttachedCycles, CallerPaysIcrc2Cycles or PatronPaysIcrc2Cycles); \
+                 token payments do not credit the wrapper's cycle balance."
+            ),
+            BridgeError::ForwardExceedsFee { fee, forward } => write!(
+                f,
+                "Cycles to forward ({forward}) exceed the fee charged ({fee}). \
+                 The fee must cover the forwarded cycles."
+            ),
         }
     }
 }

src/wrapper/tests/it/bridge_tests.rs

@@ -22,6 +22,58 @@ fn bridge_call_fails_if_target_is_self() {
     assert!(err.contains("Self-calls are not allowed through the bridge."));
 }
 
+#[test]
+fn bridge_call_rejects_forward_exceeding_fee() {
+    // Regression test for the cycle-drain: a caller must not be able to forward
+    // more cycles than the fee it pays. Here the classic attack -- a zero fee
+    // with a large forward amount -- must be rejected before any forwarding.
+    let setup = TestSetup::default();
+    let args = Call0Args {
+        target: setup.target.canister_id(),
+        method: "any_method".to_string(),
+        fee_amount: 0,
+        payment: Some(PaymentType::AttachedCycles),
+        cycles_to_forward: Some(1_000_000_000_000),
+    };
+
+    let result: Result<Result<Vec<u8>, String>, String> =
+        setup.wrapper.update(setup.user, "call0", args);
+    let inner_result = result.expect("Failed to reach canister");
+    let err = inner_result.expect_err("Should have rejected forward exceeding fee");
+    assert!(
+        err.contains("exceed the fee charged"),
+        "unexpected error: {err}"
+    );
+}
+
+#[test]
+fn bridge_call_rejects_forward_with_token_payment() {
+    // Token payments credit a token account, not the wrapper's cycle balance, so
+    // forwarding cycles against them would drain the wrapper. Even with a fee
+    // that nominally covers the forward, a token payment type must be rejected.
+    let setup = TestSetup::default();
+    let args = Call0Args {
+        target: setup.target.canister_id(),
+        method: "any_method".to_string(),
+        fee_amount: 1_000_000_000_000,
+        payment: Some(PaymentType::CallerPaysIcrc2Tokens(
+            ic_papi_api::caller::CallerPaysIcrc2Tokens {
+                ledger: Principal::anonymous(),
+            },
+        )),
+        cycles_to_forward: Some(1_000),
+    };
+
+    let result: Result<Result<Vec<u8>, String>, String> =
+        setup.wrapper.update(setup.user, "call0", args);
+    let inner_result = result.expect("Failed to reach canister");
+    let err = inner_result.expect_err("Should have rejected forward with token payment");
+    assert!(
+        err.contains("requires a cycle-denominated payment type"),
+        "unexpected error: {err}"
+    );
+}
+
 #[test]
 fn bridge_call_fails_if_insufficient_cycles() {
     let setup = TestSetup::default();