Skip to content

security(zcash): shielded send/scan hardening for v1.4.6#278

Merged
BitHighlander merged 3 commits into
developfrom
security/zcash-1.4.6-hardening
Jun 21, 2026
Merged

security(zcash): shielded send/scan hardening for v1.4.6#278
BitHighlander merged 3 commits into
developfrom
security/zcash-1.4.6-hardening

Conversation

@BitHighlander

Copy link
Copy Markdown
Collaborator

Summary

Hardens the Zcash shielded send + scanning path ahead of the v1.4.6 release. Combines the parallel security review's fixes with five release-gate fixes from the master..develop security audit, plus a transitive TLS CVE bump.

Parallel review (zcash-cli + backend)

  • Fail-closed shielded balance — RPC/UI/REST prove the cached Orchard FVK belongs to the connected device before showing/returning a balance (anti-bleed across device/seed swaps).
  • Hybrid finalize validation — shield/deshield finalize now runs the same fail-closed local Orchard validation as z→z sends via a shared validate_hybrid_orchard_consensus helper (proof verify + BatchValidator + Orchard-digest match + consensus-sighash equality); transparent digest computed per flow.
  • Account-0 + device-FVK match before shield/deshield signing.
  • Supporting refactor of scanner, wallet DB, and ZIP-244 sighash code.

Release-gate audit fixes

  • broadcast timeout — 15s per-node send_transaction timeout so a hung lightwalletd can't strand an already-signed tx; record + try next node.
  • sidecar teardown race — send-in-flight counter stops the background device-verify from stop/wipe/restart-ing the sidecar under an in-flight send build.
  • /build device gate/api/zcash/shielded/build now runs the device-FVK preflight /send enforces (every PCZT-building entry point proves device identity).
  • force re-derive on spend — spend path re-derives the device FVK every send, past the session-sticky verified flag (closes the same-handle passphrase / hidden-wallet bleed).
  • deshield Merkle-root guard — per-spend orchard_path.root == anchor guard, matching build_pczt.

Dependencies

  • rustls-webpki 0.103.9 → 0.103.13 (RUSTSEC-2026-0049/0098/0099/0104; transitive via tonic TLS). Cargo.lock only.

Test plan

  • cargo check (zcash-cli) — clean
  • tsc --noEmit (vault) — clean
  • cargo audit — 0 vulnerabilities (3 unmaintained/unsound warnings, unreachable here)
  • Device smoke (real KeepKey): shielded send / shield / deshield
  • Comprehensive multi-agent re-audit of the full shipping diff — in progress; verdict posted as a comment before this is marked ready.

Notes

  • Branched off f1a206a5 (feat(zcash): address book in private send), which was unpushed on develop and rides along.
  • Draft until the re-audit clears and device smoke passes.

BitHighlander and others added 3 commits June 20, 2026 18:19
- Remove the inline overview block from the Zcash privacy tab: the quick-action
  buttons (Send/Receive/Shield) duplicated the nav tabs, and the recent-activity
  list belongs only on the History tab.
- Wire the address book into the Zcash Send and Unshield "To" fields, reusing the
  shared AddressBookPicker, SaveRecipientDialog, AddressIdenticon, and matchAddress
  flow (new-address auto-save prompt + matched-contact badge — same UX as SendForm).
- Mark recipients private (u1… unified/Orchard) vs transparent (t1…/t3…) and gate
  by context: the transparent-only Unshield field never offers a private address,
  and vice versa, so the two pools can't be conflated.
- AddressBookPicker gains generic optional entryFilter + renderTag props to support
  the type gating without Zcash-specific logic leaking into the shared component.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Combines the parallel security review's shielded-path hardening with five
release-gate fixes from the master..develop audit, plus a transitive TLS
CVE bump. Targets the v1.4.6 release.

Parallel review (zcash-cli + backend):
- Fail-closed shielded balance — RPC, UI and REST prove the cached Orchard
  FVK belongs to the connected device before showing/returning a balance
  (anti-bleed across device/seed swaps).
- Hybrid shield/deshield finalize now runs the same fail-closed local
  Orchard validation as z->z sends, via a shared
  validate_hybrid_orchard_consensus helper: proof verify + BatchValidator +
  Orchard-digest match + consensus-sighash equality, with the transparent
  sighash digest computed per flow (shield: inputs+outputs, deshield:
  outputs only).
- Account-0 enforcement + device-FVK match before shield/deshield signing.
- Supporting refactor of the scanner, wallet DB and ZIP-244 sighash code.

Release-gate audit fixes:
- broadcast: 15s timeout on each lightwalletd send_transaction so a hung
  node can't strand an already-signed tx; record the error and try the next
  node.
- sidecar teardown race: a send-in-flight counter stops the fire-and-forget
  background device-verify from stop/wipe/restart-ing the sidecar (and
  dropping its in-memory PCZT state) underneath an in-flight send build.
- /api/zcash/shielded/build now runs the device-FVK preflight that /send
  enforces, so every PCZT-building entry point proves device identity.
- spend path re-derives the device FVK every send (force), bypassing the
  session-sticky verified flag so a same-handle passphrase / hidden-wallet
  toggle cannot build against the previous wallet's notes.
- build_deshield_pczt: per-spend Orchard MerklePath root==anchor guard,
  matching build_pczt.

Dependencies:
- rustls-webpki 0.103.9 -> 0.103.13 (RUSTSEC-2026-0049/0098/0099/0104;
  transitive via tonic TLS). Cargo.lock only.
…, DoS, validation)

Follow-up to the full-shipping-diff re-audit and reviewer pass on this branch.

P1:
- Balance bleed (FS-1): reset zcashDeviceVerified + zcashVerifiedThisSession in
  resetSeedManagers() — the single choke point for every wallet-change reset
  path, incl. the needs_passphrase handler and the result-based reconcile — so a
  same-handle passphrase / hidden-wallet toggle drops the sticky flag and the
  display/verify gates fail-closed instead of surfacing the previous wallet's
  shielded balance.
- Scan-endpoint bypass: zcashShieldedScan (RPC) and /api/zcash/shielded/scan
  (REST) now prove the cached FVK belongs to the connected device before
  returning balance, matching /balance.

P2:
- Purge race (FIXB-02): coalesce the sidecar purge behind a single in-flight
  promise so a forced spend-path re-derive and a background verify can't run
  overlapping stop/wipe/start/init cycles.
- Split-lifecycle guard: buildShieldedTx + finalizeShieldedTx now hold the
  send-in-flight counter so the split REST /build and /finalize paths are guarded
  against a background sidecar teardown.
- REST MAX_MONEY bound: /send, /deshield, /shield reject amounts above 21M ZEC
  (matches ZcashBuildRequest), closing the unbounded-amount path to the builder.
- Raw-tx parser DoS: cap the Orchard-action Vec pre-allocation by the remaining
  buffer length so a malicious lightwalletd compact_size can't OOM the sidecar.

P3 / info:
- checked_add for amount + fee in all three build flows (can't silently wrap).
- Reject (don't truncate) memos over the 512-byte ZIP-302 limit.
- REST display-address routes through emuWrap for emulator-confirm parity.
- get_address_utxos errors on a malformed txid instead of a silent zero-fill.
@BitHighlander
BitHighlander marked this pull request as ready for review June 21, 2026 00:29
@BitHighlander
BitHighlander merged commit 0b3b91d into develop Jun 21, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant