Scope Zcash shielded signing to selected account
What changed, and why it matters
This commit fixes a bug in the Keystone 3 hardware wallet's Zcash shielded transaction signing. Previously, when a user reviewed and approved a transaction for one account, the device could accidentally authorize a spend from a different account controlled by the same seed. The patch now checks that every shielded spend belongs to the account the user actually selected, and rejects the transaction if any spend comes from another account. This prevents an attacker or buggy wallet software from tricking the user into signing a spend from an account they did not review.
Treat this as a security-relevant bug fix. Users should upgrade to the firmware version containing this commit. Wallet software interacting with Keystone should ensure it passes the correct reviewed account index to the signing path and does not rely on the unscoped raw `sign_pczt` API for user-facing transactions. Reviewers should verify that the new account checks cannot be bypassed via dummy notes or missing ZIP-32 derivation data.
Security signals we found
Account-scoping enforcement added to shielded spend authorization
Prevents cross-account signing under same seed
New validation in PCZT check path before signing
New validation in PCZT sign path during spend authorization
Test coverage added for rejected unselected-account spends and accepted selected-account spends
Changelog describes bug fix for shielded signing using non-selected account
Evidence from the diff
The patch scopes Zcash PCZT (Partially Created Zcash Transaction) signing to the selected account. In rust/apps/zcash/src/pczt/check.rs, it adds a check during spend validation: if a non-dummy spend’s ZIP-32 derivation matches the seed fingerprint but belongs to a different account index, check_pczt_cypherpunk now returns ZcashError::PcztNoMyInputs. In rust/apps/zcash/src/pczt/sign.rs, SeedSigner gains an optional selected_account field; when set, any shielded spend whose derivation resolves to a different account index is rejected with PcztNoMyInputs. The sign_and_redact_pczt_with_cache helper now accepts this account scope, and the production signing path in lib.rs passes the reviewed account index. The raw sign_pczt helper keeps None to preserve its unscoped API. Tests are added/updated to verify that unselected-account spends are rejected and selected-account spends still work for both Orchard and Ironwood pools.
Changed components
rust/apps/zcash/src/lib.rsrust/apps/zcash/src/pczt/check.rsrust/apps/zcash/src/pczt/sign.rsrust/apps/zcash/src/pczt/mod.rsZcash shielded transaction signing (Orchard and Ironwood pools)Keystone 3 firmware Zcash cypherpunk featureInspect captured patch +166 / −29
diff --git a/CHANGELOG-ZH.md b/CHANGELOG-ZH.md
index e46ac78..277e683 100644
--- a/CHANGELOG-ZH.md
+++ b/CHANGELOG-ZH.md
@@ -9,6 +9,7 @@
### Bug 修复
1. 修复响应二维码生成失败时 Zcash 签名卡住的问题
+2. 修复 Zcash 屏蔽签名可能使用非所选账户的问题
## 2.5.0(2026-6-29)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3654bbb..4c794e5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,7 @@
### Bug Fixes
1. Fixed stalled Zcash signing when response QR generation fails
+2. Restricted Zcash shielded signing to the selected account
## 2.5.0(2026-6-29)
diff --git a/rust/apps/zcash/src/lib.rs b/rust/apps/zcash/src/lib.rs
index ca53ba8..56aba2f 100644
--- a/rust/apps/zcash/src/lib.rs
+++ b/rust/apps/zcash/src/lib.rs
@@ -916,11 +916,13 @@ fn signable_action_decision<P: consensus::Parameters>(
params.network_type().coin_type(),
pool.shielded_pool(),
)?;
- if matched_account != Some(account_index) {
- if policy == ShieldedActionPolicy::Batch {
+ match matched_account {
+ Some(matched_account) if matched_account == account_index => {}
+ Some(_) => return Err(ZcashError::PcztNoMyInputs),
+ None if policy == ShieldedActionPolicy::Batch => {
return Err(ZcashError::PcztNoMyInputs);
}
- return Ok(None);
+ None => return Ok(None),
}
Ok(Some(SignableShieldedAction { pool, index }))
@@ -1208,7 +1210,8 @@ fn sign_checked_pczt_with_policy<P: consensus::Parameters>(
if policy == ShieldedActionPolicy::Batch && signable_actions.is_empty() {
return Err(ZcashError::PcztNoMyInputs);
}
- let signed = pczt::sign::sign_and_redact_pczt_with_cache(pczt, seed, ask_cache)?;
+ let signed =
+ pczt::sign::sign_and_redact_pczt_with_cache(pczt, seed, Some(account_index), ask_cache)?;
let signed = if signable_actions.is_empty() {
signed
} else {
@@ -1625,7 +1628,7 @@ mod tests {
}
#[test]
- fn test_parse_and_check_ignore_unsupported_ironwood_spend_zip32_path() {
+ fn test_parse_ignores_and_check_rejects_unsupported_ironwood_spend_zip32_path() {
let sample = pczt::test_support::sample_ironwood_pczt();
let parsed_pczt = parse_pczt_cypherpunk(
&pczt::test_support::Nu6_3Network,
@@ -1656,14 +1659,16 @@ mod tests {
&sample.seed_fingerprint,
)
.expect("parse uses seed fingerprint ownership only");
- check_pczt_cypherpunk(
- &pczt::test_support::Nu6_3Network,
- &pczt,
- &sample.ufvk_text,
- &sample.seed_fingerprint,
- 0,
- )
- .expect("check ignores non-selected shielded spend paths");
+ assert_invalid_pczt_message(
+ check_pczt_cypherpunk(
+ &pczt::test_support::Nu6_3Network,
+ &pczt,
+ &sample.ufvk_text,
+ &sample.seed_fingerprint,
+ 0,
+ ),
+ "unsupported Ironwood spend ZIP 32 derivation path",
+ );
}
}
@@ -1945,6 +1950,28 @@ mod tests {
assert_eq!(signed_actions, 2);
}
+ #[test]
+ fn test_check_and_sign_reject_unselected_account_spend() {
+ let sample = pczt::test_support::sample_migration_pczt_from_account(1);
+ let check_result = check_pczt_cypherpunk(
+ &pczt::test_support::Nu6_3Network,
+ &sample.bytes,
+ &sample.ufvk_text,
+ &sample.seed_fingerprint,
+ 0,
+ );
+ assert!(matches!(check_result, Err(ZcashError::PcztNoMyInputs)));
+
+ let sign_result = sign_checked_pczt(
+ &pczt::test_support::Nu6_3Network,
+ &sample.bytes,
+ &sample.seed,
+ &sample.seed_fingerprint,
+ 0,
+ );
+ assert!(matches!(sign_result, Err(ZcashError::PcztNoMyInputs)));
+ }
+
#[test]
fn test_sign_checked_pczt_rejects_foreign_seed() {
let sample = pczt::test_support::sample_orchard_change_pczt();
diff --git a/rust/apps/zcash/src/pczt/check.rs b/rust/apps/zcash/src/pczt/check.rs
index 9a5393a..7acd520 100644
--- a/rust/apps/zcash/src/pczt/check.rs
+++ b/rust/apps/zcash/src/pczt/check.rs
@@ -566,6 +566,20 @@ fn check_action_spend<P: consensus::Parameters>(
pool: ShieldedPool,
) -> Result<(), ZcashError> {
let pool_label = pool.label();
+ if let (Some(value), Some(zip32_derivation)) = (spend.value(), spend.zip32_derivation()) {
+ if value.inner() != 0 && zip32_derivation.seed_fingerprint() == seed_fingerprint {
+ let matched_account = super::matching_seed_supported_orchard_account(
+ seed_fingerprint,
+ Some(zip32_derivation),
+ params.network_type().coin_type(),
+ pool,
+ )?;
+ if matched_account != Some(account_index) {
+ return Err(ZcashError::PcztNoMyInputs);
+ }
+ }
+ }
+
// We can only verify the `nullifier` and `rk` fields of a spend if we know its FVK.
let can_verify_nf_rk = match (spend.value(), spend.fvk(), spend.zip32_derivation()) {
// Dummy notes use randomly-generated FVKs, so if one is already present then
diff --git a/rust/apps/zcash/src/pczt/mod.rs b/rust/apps/zcash/src/pczt/mod.rs
index d63b31e..9ef5ec6 100644
--- a/rust/apps/zcash/src/pczt/mod.rs
+++ b/rust/apps/zcash/src/pczt/mod.rs
@@ -448,17 +448,22 @@ pub(crate) mod test_support {
// Orchard spend -> Ironwood output, matching one compact-eligible transfer.
pub(crate) fn sample_migration_pczt() -> SamplePczt {
- sample_migration_pczt_with_options(0, MemoBytes::empty(), None)
+ sample_migration_pczt_with_options(0, 0, MemoBytes::empty(), None)
}
/// Builds a migration whose funded output carries the given memo.
pub(crate) fn sample_migration_pczt_with_output_memo(output_memo: MemoBytes) -> SamplePczt {
- sample_migration_pczt_with_options(0, output_memo, None)
+ sample_migration_pczt_with_options(0, 0, output_memo, None)
}
/// Builds a migration whose funded output belongs to the given account.
pub(crate) fn sample_migration_pczt_to_account(output_account: u32) -> SamplePczt {
- sample_migration_pczt_with_options(output_account, MemoBytes::empty(), None)
+ sample_migration_pczt_with_options(0, output_account, MemoBytes::empty(), None)
+ }
+
+ /// Builds a migration whose spend belongs to the given account while account 0 is selected.
+ pub(crate) fn sample_migration_pczt_from_account(spend_account: u32) -> SamplePczt {
+ sample_migration_pczt_with_options(spend_account, 0, MemoBytes::empty(), None)
}
/// Adds a zero-value output, optionally marked for ordinary display.
@@ -466,11 +471,12 @@ pub(crate) mod test_support {
memo: MemoBytes,
displayable: bool,
) -> SamplePczt {
- sample_migration_pczt_with_options(0, MemoBytes::empty(), Some((memo, displayable)))
+ sample_migration_pczt_with_options(0, 0, MemoBytes::empty(), Some((memo, displayable)))
}
/// Builds a migration sample with a configurable funded recipient and optional zero output.
fn sample_migration_pczt_with_options(
+ spend_account: u32,
output_account: u32,
output_memo: MemoBytes,
zero_output: Option<(MemoBytes, bool)>,
@@ -479,7 +485,15 @@ pub(crate) mod test_support {
let seed = [7u8; 32];
let ufvk_text = derive_ufvk(¶ms, &seed, "m/32'/133'/0'").unwrap();
let ufvk = UnifiedFullViewingKey::decode(¶ms, &ufvk_text).unwrap();
- let orchard_fvk = ufvk.orchard().unwrap().clone();
+ let selected_fvk = ufvk.orchard().unwrap().clone();
+ let spend_ufvk_text = derive_ufvk(
+ ¶ms,
+ &seed,
+ &alloc::format!("m/32'/133'/{spend_account}'"),
+ )
+ .unwrap();
+ let spend_ufvk = UnifiedFullViewingKey::decode(¶ms, &spend_ufvk_text).unwrap();
+ let orchard_fvk = spend_ufvk.orchard().unwrap().clone();
let orchard_ivk = orchard_fvk.to_ivk(orchard::keys::Scope::External);
let spend_recipient = orchard_fvk.address_at(0u32, orchard::keys::Scope::External);
let output_ufvk_text = derive_ufvk(
@@ -494,7 +508,7 @@ pub(crate) mod test_support {
// the foreign-account variant decryptable for its ordinary-review test
// by using the selected account's external OVK.
let recipient = output_fvk.address_at(0u32, orchard::keys::Scope::Internal);
- let orchard_ovk = orchard_fvk.to_ovk(if output_account == 0 {
+ let orchard_ovk = selected_fvk.to_ovk(if spend_account == 0 && output_account == 0 {
orchard::keys::Scope::Internal
} else {
orchard::keys::Scope::External
@@ -603,7 +617,7 @@ pub(crate) mod test_support {
vec![
zip32::ChildIndex::hardened(32).index(),
zip32::ChildIndex::hardened(133).index(),
- zip32::ChildIndex::hardened(0).index(),
+ zip32::ChildIndex::hardened(spend_account).index(),
],
)
.unwrap();
diff --git a/rust/apps/zcash/src/pczt/sign.rs b/rust/apps/zcash/src/pczt/sign.rs
index 366d6d1..e9b5d2e 100644
--- a/rust/apps/zcash/src/pczt/sign.rs
+++ b/rust/apps/zcash/src/pczt/sign.rs
@@ -214,6 +214,9 @@ impl Default for SpendAuthCache {
struct SeedSigner<'a> {
seed: &'a [u8],
seed_fingerprint: [u8; 32],
+ /// Restricts checked production signing to the account the user reviewed.
+ /// The raw `sign_pczt` helper passes `None` to preserve its unscoped API.
+ selected_account: Option<zcash_vendor::zip32::AccountId>,
pool: ShieldedPool,
/// Borrowed so every PCZT and both pool passes can share one scrubbed
/// slot. See [`SpendAuthCache`] for the request-scoping contract.
@@ -228,12 +231,14 @@ impl<'a> SeedSigner<'a> {
fn new(
seed: &'a [u8],
seed_fingerprint: [u8; 32],
+ selected_account: Option<zcash_vendor::zip32::AccountId>,
pool: ShieldedPool,
ask_cache: &'a SpendAuthCache,
) -> Self {
Self {
seed,
seed_fingerprint,
+ selected_account,
pool,
ask_cache,
signed: Cell::new(0),
@@ -346,6 +351,12 @@ impl PcztSigner for SeedSigner<'_> {
// Not derivable from this seed; not ours to sign.
return Ok(());
};
+ if self
+ .selected_account
+ .is_some_and(|selected_account| selected_account != account_index)
+ {
+ return Err(ZcashError::PcztNoMyInputs);
+ }
self.with_spend_authorizing_key(account_index, |ask| {
action
@@ -380,17 +391,20 @@ pub fn sign_pczt(pczt: Pczt, seed: &[u8]) -> crate::Result<Vec<u8>> {
/// cache so PCZTs for the selected account share one derivation.
#[cfg(feature = "cypherpunk")]
pub fn sign_and_redact_pczt(pczt: Pczt, seed: &[u8]) -> crate::Result<Pczt> {
- sign_and_redact_pczt_with_cache(pczt, seed, &SpendAuthCache::new())
+ sign_and_redact_pczt_with_cache(pczt, seed, None, &SpendAuthCache::new())
}
-/// [`sign_and_redact_pczt`] with a caller-provided [`SpendAuthCache`]. The normal
-/// batch path derives its selected account key once and reuses it across PCZTs
-/// and pools. An account change scrubs and replaces the slot. The cache must not
-/// be reused with another seed.
+/// [`sign_and_redact_pczt`] with a caller-provided [`SpendAuthCache`]. When
+/// `selected_account` is `Some`, every same-seed shielded authorization is
+/// restricted to that reviewed account. `None` is reserved for the raw,
+/// unscoped [`sign_pczt`] path. The normal batch path derives its selected
+/// account key once and reuses it across PCZTs and pools. An account change
+/// scrubs and replaces the slot. The cache must not be reused with another seed.
#[cfg(feature = "cypherpunk")]
pub(crate) fn sign_and_redact_pczt_with_cache(
pczt: Pczt,
seed: &[u8],
+ selected_account: Option<zcash_vendor::zip32::AccountId>,
ask_cache: &SpendAuthCache,
) -> crate::Result<Pczt> {
super::validate_supported_pczt(&pczt)?;
@@ -402,7 +416,13 @@ pub(crate) fn sign_and_redact_pczt_with_cache(
// The orchard signer handles both the transparent inputs and the Orchard bundle
// (the pool only changes error labels for shielded actions). Ironwood gets its own.
- let orchard_signer = SeedSigner::new(seed, seed_fingerprint, ShieldedPool::Orchard, ask_cache);
+ let orchard_signer = SeedSigner::new(
+ seed,
+ seed_fingerprint,
+ selected_account,
+ ShieldedPool::Orchard,
+ ask_cache,
+ );
// Propagate signer errors directly so strict path validation remains
// `InvalidPczt`.
@@ -410,8 +430,13 @@ pub(crate) fn sign_and_redact_pczt_with_cache(
let signer = pczt_ext::sign_transparent(signer, &orchard_signer)?;
let signer = pczt_ext::sign_orchard(signer, &orchard_signer)?;
- let ironwood_signer =
- SeedSigner::new(seed, seed_fingerprint, ShieldedPool::Ironwood, ask_cache);
+ let ironwood_signer = SeedSigner::new(
+ seed,
+ seed_fingerprint,
+ selected_account,
+ ShieldedPool::Ironwood,
+ ask_cache,
+ );
let signer = if process_ironwood {
pczt_ext::sign_ironwood(signer, &ironwood_signer)?
} else {
@@ -639,7 +664,7 @@ mod tests {
(ShieldedPool::Orchard, 1),
(ShieldedPool::Orchard, 0),
] {
- let signer = SeedSigner::new(&seed, fingerprint, pool, &cache);
+ let signer = SeedSigner::new(&seed, fingerprint, None, pool, &cache);
let bytes = signer
.with_spend_authorizing_key(account(i), |ask| Ok(ask_scalar_bytes(ask)))
.unwrap();
@@ -659,6 +684,7 @@ mod tests {
sign_and_redact_pczt_with_cache(
Pczt::parse(&sample.bytes).unwrap(),
&sample.seed,
+ None,
&cache,
)
.expect("shared-cache PCZT should sign");
@@ -667,6 +693,60 @@ mod tests {
assert_eq!(cache.0.borrow().account, Some(zip32::AccountId::ZERO));
}
+ #[test]
+ fn test_scoped_signer_only_signs_selected_account() {
+ let sample = crate::pczt::test_support::sample_migration_pczt_from_account(1);
+ let account_one = zip32::AccountId::try_from(1).unwrap();
+ let pczt = Pczt::parse(&sample.bytes).unwrap();
+ let sighash = RoleSigner::new(pczt.clone())
+ .expect("account-1 PCZT signer should initialize")
+ .shielded_sighash();
+ let signed = sign_and_redact_pczt_with_cache(
+ pczt,
+ &sample.seed,
+ Some(account_one),
+ &SpendAuthCache::new(),
+ )
+ .expect("account-1 spend should sign when account 1 is selected");
+ let action = signed
+ .orchard()
+ .actions()
+ .iter()
+ .find(|action| action.spend().spend_auth_sig().is_some())
+ .expect("account-1 spend must be signed");
+ let sig: orchard::primitives::redpallas::Signature<
+ orchard::primitives::redpallas::SpendAuth,
+ > = action.spend().spend_auth_sig().unwrap().into();
+ let rk = orchard::primitives::redpallas::VerificationKey::<
+ orchard::primitives::redpallas::SpendAuth,
+ >::try_from(*action.spend().rk())
+ .expect("randomized validating key must parse");
+ rk.verify(&sighash, &sig)
+ .expect("account-1 signature must match its randomized key");
+
+ let result = sign_and_redact_pczt_with_cache(
+ Pczt::parse(&sample.bytes).unwrap(),
+ &sample.seed,
+ Some(zip32::AccountId::ZERO),
+ &SpendAuthCache::new(),
+ );
+
+ assert!(matches!(result, Err(ZcashError::PcztNoMyInputs)));
+ }
+
+ #[test]
+ fn test_scoped_ironwood_signer_rejects_unselected_account() {
+ let sample = crate::pczt::test_support::sample_ironwood_pczt();
+ let result = sign_and_redact_pczt_with_cache(
+ Pczt::parse(&sample.bytes).unwrap(),
+ &sample.seed,
+ Some(zip32::AccountId::try_from(1).unwrap()),
+ &SpendAuthCache::new(),
+ );
+
+ assert!(matches!(result, Err(ZcashError::PcztNoMyInputs)));
+ }
+
fn signable_sample_pczt() -> crate::pczt::test_support::SamplePczt {
crate::pczt::test_support::sample_ironwood_pczt()
}
Why this scored 64/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.