fix: reject all shielded PCZTs in the legacy transparent-only path
What the developer wrote
Authored by Adam Tucker
fix: reject all shielded PCZTs in the legacy transparent-only path
The legacy multi_coins check/parse/sign path only handles transparent
data, but pczt_requires_cypherpunk_support only rejected V6/Ironwood PCZTs.
A v5 PCZT carrying an Orchard or Sapling bundle slipped through: its
transparent part was validated and signed while the shielded bundle was
silently ignored. Extend the guard to also reject Orchard actions and
Sapling spends/outputs so the boundary is strictly transparent-only.
What changed, and why it matters
This commit fixes a security gap in the Keystone hardware wallet's Zcash transaction handling. The older 'legacy' code path was only designed to inspect and sign transparent (non-private) Zcash transactions, but its safety check only blocked the newest V6 and Ironwood transaction types. A version 5 transaction containing hidden Sapling or Orchard transfers could slip past the check. The wallet would then validate and sign only the visible transparent portion, while silently ignoring the shielded part. That could let a malicious or buggy app trick a user into approving a transaction that also moves funds privately. The patch now rejects any transaction containing Sapling or Orchard shielded data in the legacy path, forcing it to use the newer 'cypherpunk' path that understands those formats.
Treat this as a security fix and include it in the next firmware release. Verify that builds without the cypherpunk feature correctly reject all shielded PCZTs in tests, and audit any other legacy paths that may parse or sign PCZTs to ensure they route shielded transactions to the cypherpunk implementation. Consider adding explicit regression tests for v5 PCZTs containing Sapling and Orchard bundles.
Security signals we found
Incomplete validation/signature path for shielded transaction data
Silent dropping of Sapling/Orchard bundles in legacy transparent-only code
Guard condition too narrow (only V6 + Ironwood) before patch
Potential user-interface inconsistency between displayed and signed transaction
Patch hardens rejection boundary but is partial: relies on cfg gating and correct feature builds
Evidence from the diff
The legacy multi_coins check/parse/sign path in rust/apps/zcash only handles transparent Zcash data. The guard pczt_requires_cypherpunk_support previously returned true only for tx_version >= 6 or non-empty Ironwood actions, so a v5 PCZT with Sapling spends/outputs or Orchard actions was accepted. In that case the transparent inputs/outputs were validated, parsed for display, and signed, while the shielded bundles were ignored. This creates a split-view risk: the user sees and signs only transparent activity, but the final transaction may also spend or transfer shielded value. The patch extends pczt_requires_cypherpunk_support to also detect non-empty Sapling spends, Sapling outputs, and Orchard actions, and updates error messages and tests accordingly. The fix is gated by the zcash_unstable = “nu6.3” cfg and applies when the cypherpunk feature is absent or multi_coins is present.
Changed components
rust/apps/zcash/src/lib.rs (legacy check path)rust/apps/zcash/src/pczt/mod.rs (pczt_requires_cypherpunk_support guard)rust/apps/zcash/src/pczt/parse.rs (legacy parse path)rust/apps/zcash/src/pczt/sign.rs (legacy sign path)Inspect captured patch +21 / −12
diff --git a/rust/apps/zcash/src/lib.rs b/rust/apps/zcash/src/lib.rs
index 6b04acc..e6469b4 100644
--- a/rust/apps/zcash/src/lib.rs
+++ b/rust/apps/zcash/src/lib.rs
@@ -139,11 +139,12 @@ fn transparent_account_pubkey_from_xpub(
fn reject_legacy_check_unsupported_pczt(pczt: &Pczt) -> Result<()> {
#[cfg(zcash_unstable = "nu6.3")]
{
- // The legacy multi-coins check path only verifies transparent data.
- // Reject V6/Ironwood PCZTs so check, parse, and sign enforce the same boundary.
+ // The legacy multi-coins check path only verifies transparent data. Reject any
+ // shielded (Sapling/Orchard/Ironwood) or V6 PCZT so check, parse, and sign
+ // enforce the same transparent-only boundary.
if pczt::pczt_requires_cypherpunk_support(pczt) {
return Err(ZcashError::InvalidPczt(
- "V6 or Ironwood PCZTs require cypherpunk checking support".to_string(),
+ "Shielded or V6 PCZTs require cypherpunk checking support".to_string(),
));
}
}
@@ -318,7 +319,7 @@ mod legacy_tests {
assert!(matches!(
result,
Err(ZcashError::InvalidPczt(msg))
- if msg == "V6 or Ironwood PCZTs require cypherpunk checking support"
+ if msg == "Shielded or V6 PCZTs require cypherpunk checking support"
));
}
}
diff --git a/rust/apps/zcash/src/pczt/mod.rs b/rust/apps/zcash/src/pczt/mod.rs
index 0a8c154..8c0bb0c 100644
--- a/rust/apps/zcash/src/pczt/mod.rs
+++ b/rust/apps/zcash/src/pczt/mod.rs
@@ -161,12 +161,19 @@ pub(crate) fn matching_seed_supported_orchard_account(
.map_err(|_| unsupported_path())
}
+/// Returns whether a PCZT carries anything the transparent-only legacy path
+/// cannot handle: a v6+ transaction, or any shielded (Sapling/Orchard/Ironwood)
+/// content. These must be checked, parsed, and signed by the cypherpunk build.
#[cfg(all(
zcash_unstable = "nu6.3",
any(feature = "multi_coins", not(feature = "cypherpunk"))
))]
pub(crate) fn pczt_requires_cypherpunk_support(pczt: &zcash_vendor::pczt::Pczt) -> bool {
- *pczt.global().tx_version() >= 6 || !pczt.ironwood().actions().is_empty()
+ *pczt.global().tx_version() >= 6
+ || !pczt.sapling().spends().is_empty()
+ || !pczt.sapling().outputs().is_empty()
+ || !pczt.orchard().actions().is_empty()
+ || !pczt.ironwood().actions().is_empty()
}
#[cfg(all(test, feature = "cypherpunk"))]
diff --git a/rust/apps/zcash/src/pczt/parse.rs b/rust/apps/zcash/src/pczt/parse.rs
index 6a06ab0..4a018cd 100644
--- a/rust/apps/zcash/src/pczt/parse.rs
+++ b/rust/apps/zcash/src/pczt/parse.rs
@@ -380,11 +380,12 @@ pub fn parse_pczt_multi_coins<P: consensus::Parameters>(
fn reject_legacy_parse_unsupported_pczt(pczt: &Pczt) -> Result<(), ZcashError> {
#[cfg(zcash_unstable = "nu6.3")]
{
- // The legacy multi-coins parser only displays transparent data. Reject
- // V6/Ironwood PCZTs instead of showing an incomplete transaction review.
+ // The legacy multi-coins parser only displays transparent data. Reject any
+ // shielded (Sapling/Orchard/Ironwood) or V6 PCZT instead of showing an
+ // incomplete transaction review.
if super::pczt_requires_cypherpunk_support(pczt) {
return Err(ZcashError::InvalidPczt(
- "V6 or Ironwood PCZTs require cypherpunk parsing support".to_string(),
+ "Shielded or V6 PCZTs require cypherpunk parsing support".to_string(),
));
}
}
@@ -793,7 +794,7 @@ mod legacy_tests {
assert!(matches!(
result,
Err(ZcashError::InvalidPczt(msg))
- if msg == "V6 or Ironwood PCZTs require cypherpunk parsing support"
+ if msg == "Shielded or V6 PCZTs require cypherpunk parsing support"
));
}
}
diff --git a/rust/apps/zcash/src/pczt/sign.rs b/rust/apps/zcash/src/pczt/sign.rs
index 62bfdbf..233cbde 100644
--- a/rust/apps/zcash/src/pczt/sign.rs
+++ b/rust/apps/zcash/src/pczt/sign.rs
@@ -145,10 +145,10 @@ fn reject_legacy_unsupported_pczt(pczt: &Pczt) -> Result<(), ZcashError> {
#[cfg(zcash_unstable = "nu6.3")]
{
// The legacy helper below carries the pre-NU6.3 transparent sighash implementation.
- // It must not be used for V6/Ironwood PCZTs.
+ // It must not be used for shielded (Sapling/Orchard/Ironwood) or V6 PCZTs.
if super::pczt_requires_cypherpunk_support(pczt) {
return Err(ZcashError::SigningError(
- "V6 or Ironwood PCZTs require cypherpunk signing support".to_string(),
+ "Shielded or V6 PCZTs require cypherpunk signing support".to_string(),
));
}
}
@@ -699,7 +699,7 @@ mod legacy_tests {
assert!(matches!(
result,
Err(ZcashError::SigningError(msg))
- if msg == "V6 or Ironwood PCZTs require cypherpunk signing support"
+ if msg == "Shielded or V6 PCZTs require cypherpunk signing support"
));
}
}
Why this scored 72/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.