fix(zcash): validate empty shielded bundle balances
What changed, and why it matters
This commit adds a validation check for Zcash PCZT (Partially Created Zcash Transaction) parsing in the Keystone hardware wallet firmware. It ensures that if an Orchard or Ironwood shielded bundle contains no actions, its declared value sum must be zero. Previously, a malformed or malicious transaction with an empty shielded bundle but a non-zero value sum might have been accepted, potentially allowing incorrect balance reporting or transaction signing.
Review whether this validation is sufficient and whether similar consistency checks are needed for Sapling and transparent bundles. Ensure the fix is included in the next firmware release and consider whether any prior firmware version accepted such malformed PCZTs.
Security signals we found
Input validation added for empty shielded bundle value_sum
Potential balance-consistency issue in Zcash PCZT handling
Test case demonstrates malformed PCZT rejection
Fix targets Orchard and Ironwood shielded pools
Evidence from the diff
The patch introduces validate_empty_orchard_protocol_bundle_balances() in rust/apps/zcash/src/pczt/mod.rs. It iterates over Orchard and Ironwood bundles and rejects any PCZT where bundle.actions().is_empty() but bundle.value_sum().0 != 0. A test is added that crafts an empty shielded bundle with value_sum: (1, false) and confirms validation fails with ZcashError::InvalidPczt. This is a defensive consistency check, not a full fix for an active exploit.
Changed components
rust/apps/zcash/src/pczt/mod.rsZcash PCZT validation logicOrchard shielded pool parserIronwood shielded pool parserInspect captured patch +99 / −0
diff --git a/rust/apps/zcash/src/pczt/mod.rs b/rust/apps/zcash/src/pczt/mod.rs
index 56b78ee..ace2e67 100644
--- a/rust/apps/zcash/src/pczt/mod.rs
+++ b/rust/apps/zcash/src/pczt/mod.rs
@@ -19,6 +19,7 @@ pub(crate) fn parse_pczt(bytes: &[u8]) -> Result<Pczt, ZcashError> {
pub(crate) fn validate_supported_pczt(pczt: &Pczt) -> Result<(), ZcashError> {
validate_sapling_bundle_consistency(pczt)?;
+ validate_empty_orchard_protocol_bundle_balances(pczt)?;
{
if pczt_has_ironwood_actions(pczt) && !pczt_is_v6(pczt) {
@@ -48,6 +49,18 @@ pub(crate) fn validate_supported_pczt(pczt: &Pczt) -> Result<(), ZcashError> {
Ok(())
}
+fn validate_empty_orchard_protocol_bundle_balances(pczt: &Pczt) -> Result<(), ZcashError> {
+ for (pool, bundle) in [("Orchard", pczt.orchard()), ("Ironwood", pczt.ironwood())] {
+ if bundle.actions().is_empty() && bundle.value_sum().0 != 0 {
+ return Err(ZcashError::InvalidPczt(format!(
+ "{pool} value_sum must be zero when {pool} bundle is empty"
+ )));
+ }
+ }
+
+ Ok(())
+}
+
/// Ensures every Orchard protocol action has a distinct randomized validating key.
///
/// Within each pool, spend authorization signatures cover the same transaction-wide
@@ -234,6 +247,92 @@ pub(crate) fn pczt_requires_cypherpunk_support(pczt: &zcash_vendor::pczt::Pczt)
|| !pczt.ironwood().actions().is_empty()
}
+#[cfg(all(test, feature = "cypherpunk"))]
+mod consistency_tests {
+ use alloc::{format, vec::Vec};
+
+ use ::pczt::roles::creator::Creator;
+ use serde::{Deserialize, Serialize};
+ use zcash_vendor::zcash_protocol::consensus::{BranchId, MainNetwork, NetworkConstants};
+
+ use super::*;
+
+ #[derive(Serialize, Deserialize)]
+ struct EmptyPcztWire {
+ global: ::pczt::common::Global,
+ transparent: Option<::pczt::transparent::Bundle>,
+ sapling: Option<::pczt::sapling::Bundle>,
+ orchard: Option<EmptyShieldedBundleWire>,
+ ironwood: Option<EmptyShieldedBundleWire>,
+ }
+
+ #[derive(Serialize, Deserialize)]
+ struct EmptyShieldedBundleWire {
+ actions: Vec<()>,
+ flags: u8,
+ value_sum: (u64, bool),
+ anchor: Option<[u8; 32]>,
+ note_version: NoteVersionWire,
+ zkproof: Option<Vec<u8>>,
+ bsk: Option<[u8; 32]>,
+ }
+
+ #[derive(Serialize, Deserialize)]
+ enum NoteVersionWire {
+ V2,
+ V3,
+ }
+
+ fn empty_bundle_with_nonzero_value_sum(pool: ShieldedPool) -> Vec<u8> {
+ let branch_id = match pool {
+ ShieldedPool::Orchard => BranchId::Nu6,
+ ShieldedPool::Ironwood => BranchId::Nu6_3,
+ };
+ let bytes = Creator::new(branch_id.into(), 10, MainNetwork.coin_type(), None, None)
+ .unwrap()
+ .build()
+ .unwrap()
+ .serialize()
+ .unwrap();
+ let mut wire: EmptyPcztWire = postcard::from_bytes(&bytes[8..]).unwrap();
+ let bundle = EmptyShieldedBundleWire {
+ actions: Vec::new(),
+ flags: match pool {
+ ShieldedPool::Orchard => 0b0000_0011,
+ ShieldedPool::Ironwood => 0b0000_0111,
+ },
+ value_sum: (1, false),
+ anchor: None,
+ note_version: match pool {
+ ShieldedPool::Orchard => NoteVersionWire::V2,
+ ShieldedPool::Ironwood => NoteVersionWire::V3,
+ },
+ zkproof: None,
+ bsk: None,
+ };
+ match pool {
+ ShieldedPool::Orchard => wire.orchard = Some(bundle),
+ ShieldedPool::Ironwood => wire.ironwood = Some(bundle),
+ }
+
+ postcard::to_extend(&wire, bytes[..8].to_vec()).unwrap()
+ }
+
+ #[test]
+ fn rejects_nonzero_value_sum_on_empty_orchard_protocol_bundles() {
+ for pool in [ShieldedPool::Orchard, ShieldedPool::Ironwood] {
+ let pczt = parse_pczt(&empty_bundle_with_nonzero_value_sum(pool)).unwrap();
+
+ assert_eq!(
+ validate_supported_pczt(&pczt),
+ Err(ZcashError::InvalidPczt(format!(
+ "{pool} value_sum must be zero when {pool} bundle is empty"
+ )))
+ );
+ }
+ }
+}
+
#[cfg(all(test, feature = "cypherpunk"))]
pub(crate) mod test_support {
use alloc::{string::String, vec, vec::Vec};
Why this scored 59/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.