Orchard spending key survives device lock: no Initialize/Cancel/ClearSession/auto-lock path calls zcash_signing_abort(), contrary to the comment that says they do

[BitHighlander]

Finding

Severity: high · Dimension: memory-safety · Location: lib/firmware/fsm_msg_common.h:624

1. Host sends ZcashSignPCZT. User enters the PIN and holds through the summary screen. fsm_msg_zcash.h:791-793 sets zcash_signing.active = true and zcash_signing.keys now holds sk/ask/nk/rivk/dk for the account — full Orchard spend authority.
2. Before the flow completes, either (a) the host sends Cancel, Initialize or ClearSession, or (b) the user walks away and the screensaver fires (home_sm.c:123, session_clear(clear_pin=true)).
3. session_clear() wipes session.seed, session.passphrase, the bip32/bip39 caches and pinCached; setup_abort/signing_abort/ethereum_signing_abort/tendermint_signAbort/eos_signingAbort clear every other engine. zcash_signing is untouched: active stays true and keys.sk / keys.ask stay populated.

Two consequences. First, secret hygiene: the device presents as locked while a live Orchard spending key sits in ordinary .bss for the rest of the boot — not even in the confidential NOLOAD section the bootloader clears at boot (CMakeLists.txt:107, tools/bootloader/main.c:81-83), unlike static SessionState CONFIDENTIAL session (storage.c:92) and static ConfigFlash CONFIDENTIAL shadow_config (storage.c:105). Second, the lock does not actually stop signing: fsm_msgZcashPCZTAction has no CHECK_PIN, so after the auto-lock the host can keep streaming ZcashPCZTAction messages and the locked device keeps producing RedPallas spend authorizations from the retained key (fsm_msg_zcash.h:1211-1216). A Cancel likewise does not cancel — the session is resumable, which is not true of any other engine.

Evidence

Every other coin engine is torn down; Zcash is missing from all four lists.

lib/firmware/fsm_msg_common.h:624-633
  void fsm_msgCancel(Cancel* msg) {
    (void)msg;
    setup_abort();
    signing_abort();
    ethereum_signing_abort();
    tendermint_signAbort();
    eos_signingAbort();                <-- no zcash_signing_abort()
    fsm_sendFailure(FailureType_Failure_ActionCancelled, "Aborted");
  }

lib/firmware/fsm_msg_common.h:1-12  (fsm_msgInitialize) — same five calls, then session_clear(false); no zcash_signing_abort().
lib/firmware/fsm.c:307-311           void fsm_msgClearSession(...) { session_clear(true); fsm_sendSuccess(...); }
lib/firmware/home_sm.c:119-124       auto-lock: if (idle_time >= storage_getAutoLockDelayMs()) { session_clear(true); layout_screensaver(); ... }

The comment asserts the opposite — lib/firmware/fsm.c:355-360:
  // Zcash shielded/Orchard engine compiled out. The always-on
  // Initialize/ClearSession/Cancel handlers still call zcash_signing_abort();
  // with no privacy state to reset, a no-op is correct.
  void zcash_signing_abort(void) {}

Exhaustive grep over the tree (excluding deps/) shows every call site is inside fsm_msg_zcash.h itself; nothing in fsm.c, fsm_msg_common.h, storage.c or home_sm.c calls it.

What is retained — lib/firmware/fsm_msg_zcash.h:67-76:
  /* Zcash shielded signing state */
  static struct {          <-- note: NO CONFIDENTIAL attribute
    bool active;
    ...
    ZcashOrchardKeys keys;

include/keepkey/firmware/zcash.h:31-44:
    uint8_t sk[32];   /* Spending key (master secret at this level) */
    uint8_t ask[32];  /* Spend authorizing key (scalar) */
    uint8_t nk[32]; uint8_t rivk[32]; uint8_t dk[32];

What session_clear DOES wipe — lib/firmware/storage.c:1710-1717, 1757-1763: signed_metadata_clear_signers(), bip32/bip39 caches, ss->seed, ss->passphrase, pinCached.

And the continuation handlers carry no PIN gate: fsm_msg_zcash.h:1043-1049 (fsm_msgZcashPCZTAction), :1312 and :1433 open with only `if (!zcash_signing.active)`. Only fsm_msgZcashSignPCZT has CHECK_PIN (fsm_msg_zcash.h:596-598).

Suggested fix

Add zcash_signing_abort() to fsm_msgCancel (fsm_msg_common.h:631), fsm_msgInitialize (fsm_msg_common.h:8) and fsm_msgClearSession (fsm.c:309) — or, better, call it from session_clear() in storage.c:1710 alongside signed_metadata_clear_signers(), which covers the auto-lock path in home_sm.c:123 too. Also mark the state struct static struct {...} CONFIDENTIAL zcash_signing; so its key material lands in the bootloader-wiped section, and either fix or delete the now-false comment at fsm.c:356-359.

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 ?.