Production Orchard spend-authorization signs for a host-supplied rk that is never checked against the device's own ak

[BitHighlander]

Finding

Severity: medium · Dimension: zcash-crypto-binding · Location: deps/crypto/trezor-firmware/crypto/redpallas.c:382

Host sends a ZcashPCZTAction with is_spend=true and any 32 bytes for rk. is_spend is host-asserted and is bound into no digest (fsm_msg_zcash.h:1150-1161 hashes nullifier/cmx/epk/enc_/cv_net/rk/out_ciphertext, never is_spend), and rk is never compared against ak + [alpha]G_spendauth, so the device has no on-device criterion at all for deciding whether the spend it is authorizing is one of its own notes. Concretely this is the missing gate for the Ironwood attack above: with the check in place the device would derive rk_i = ak + [alpha_i]G for each streamed action and could refuse when the same rk is presented twice in one session -- exactly the condition that makes the single emitted signature reusable in a bundle the user never saw. As it stands the device is a signing oracle that emits s = r + H(R||rk||M)(ask + alpha) for attacker-chosen rk and alpha; an individual signature is only valid when rk really equals ak + [alpha]G, so the standalone impact is limited, but the check the code already caches keys.ak for is simply not performed.

Evidence

redpallas.c:382-411 -- the function the firmware actually calls; rk is only ever hashed, never validated:
int redpallas_sign_digest_for_rk(const uint8_t* ask, const uint8_t* alpha,
                                 const uint8_t* rk, const uint8_t* sighash, ...) {
  ...
  bn_read_le(ask, &ask_scalar);
  bn_read_le(alpha, &alpha_scalar);
  bn_copy(&ask_scalar, &rsk);
  pallas_ct_add_mod_q(&rsk, &alpha_scalar);
  const int result = redpallas_sign_with_rsk(
      &rsk, rk, sighash, T, sig_out, progress, progress_context, 0, 1000);

redpallas.c:436-452 -- the checked variant, in the same file, used ONLY by unit tests:
int redpallas_sign_digest_with_ak(const uint8_t* ask, const uint8_t* ak, ...) {
  if (redpallas_derive_rk_from_ak(ak, alpha, rk_bytes) != 0 ||
      memcmp(rk_bytes, expected_rk, sizeof(rk_bytes)) != 0) {
    memzero(rk_bytes, sizeof(rk_bytes));
    return -1;
  }

fsm_msg_zcash.h:1210-1213 -- production call site, passes msg->rk straight through:
    int sign_rc = redpallas_sign_digest_for_rk(
        zcash_signing.keys.ask, msg->alpha.bytes, msg->rk.bytes, sighash,
        zcash_T, zcash_signing.signatures[zcash_signing.signature_count],
        zcash_action_progress, &signing_progress);

zcash.c:693-696 -- ak is cached in production for exactly this check, then never used for it:
  /* Cache the public key produced by the normalization multiplication.  This
   * ... lets signing derive rk from public ak + public alpha. */
  memcpy(keys->ak, ak_bytes, sizeof(keys->ak));

unittests/firmware/zcash.cpp:1714-1718 -- the gap is pinned as intended behaviour:
  ASSERT_EQ(redpallas_sign_digest_for_rk(keys.ask, alpha, wrong_rk, sighash,
                                         kRedPallasTestT,
                                         signature, nullptr, nullptr),
            0);

Suggested fix

Call redpallas_sign_digest_with_ak(zcash_signing.keys.ask, zcash_signing.keys.ak, msg->alpha.bytes, msg->rk.bytes, sighash, zcash_T, ...) at fsm_msg_zcash.h:1210 instead of redpallas_sign_digest_for_rk, so a wrong rk aborts the session, and additionally track the rk values already signed in this session and refuse a duplicate.

Verification

(not independently verified)

---

Found by an adversarial audit of release/7.15 (628b09257). Each finding was independently re-checked by a separate reviewer instructed to refute it by default; this one survived at confidence ?.