Reject existing Zcash batch signatures
What changed, and why it matters
This firmware update changes how Keystone hardware wallets handle Zcash batch signing. Previously, the device might have accepted a transaction request that already contained spend authorization signatures. Now it rejects such requests. The concern is that an attacker could craft a Zcash batch transaction containing signatures from another source and trick the device into including them in its final response, potentially making the device vouch for transactions the user did not actually review and authorize. The patch adds an explicit check to block any incoming Orchard or Ironwood spend authorization signatures in batch mode.
Treat this commit as a security-hardening fix. Ensure the firmware release containing this change is deployed to devices using Zcash batch signing. Review whether prior firmware versions could be coerced into propagating host-supplied signatures in batch responses, and consider whether any advisory or CVE is warranted if a practical exploit path exists.
Security signals we found
Defensive input validation added to reject pre-existing spend authorization signatures in Zcash batch PCZT flow
Potential host-supplied signature smuggling vector in batch signing mitigated
New unit tests cover both Orchard and Ironwood pools for the rejection behavior
CHANGELOG language clarified to emphasize device-generated compact signatures
Evidence from the diff
The commit modifies rust/apps/zcash/src/lib.rs to extend reject_unsupported_batch_pczt(). It now iterates over Orchard and Ironwood action bundles and rejects the PCZT if any spend action already has a spend_auth_sig set. The doc comments for sign_checked_batch_pczt are updated to note this new restriction. New unit tests create partially signed sample PCZTs and verify that both check_batch_pczt_with_display() and sign_checked_batch_pczt() return InvalidPczt errors. The CHANGELOG wording is also tightened to clarify that compact signature results are generated by the device. The patch is defensive: it prevents host-supplied signatures from being carried into the device’s batch signing response, which could otherwise let a malicious host smuggle unauthorized signatures into a response that the verifier treats as device-authorized.
Changed components
rust/apps/zcash/src/lib.rsZcash batch PCZT signing flowOrchard and Ironwood spend authorization signature handlingInspect captured patch +126 / −3
diff --git a/CHANGELOG-ZH.md b/CHANGELOG-ZH.md
index 23140f4..a1ede59 100644
--- a/CHANGELOG-ZH.md
+++ b/CHANGELOG-ZH.md
@@ -4,7 +4,7 @@
### 新增
-1. 支持 Zcash 批量 PCZT 签名
+1. 支持 Zcash 批量 PCZT 签名,并返回由设备生成的紧凑签名结果
### Bug 修复
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ba40735..9b4b8d5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,7 +5,7 @@
### What's new
1. Added support for Zcash batch PCZT signing with compact signature results
- that report the signing firmware version
+ generated by the device that report the signing firmware version
### Bug Fixes
diff --git a/rust/apps/zcash/src/lib.rs b/rust/apps/zcash/src/lib.rs
index cfe1768..6e46d2b 100644
--- a/rust/apps/zcash/src/lib.rs
+++ b/rust/apps/zcash/src/lib.rs
@@ -928,6 +928,21 @@ fn reject_unsupported_batch_pczt(pczt: &Pczt) -> Result<()> {
));
}
+ // A batch response contains signatures produced by this device for the
+ // reviewed request. Reject incoming signatures instead of carrying
+ // bytes supplied by the host into that response.
+ for (pool, bundle) in [("Orchard", pczt.orchard()), ("Ironwood", pczt.ironwood())] {
+ if bundle
+ .actions()
+ .iter()
+ .any(|action| action.spend().spend_auth_sig().is_some())
+ {
+ return Err(ZcashError::InvalidPczt(alloc::format!(
+ "Zcash batch request must not contain {pool} spend authorization signatures"
+ )));
+ }
+ }
+
Ok(())
}
@@ -1197,7 +1212,8 @@ pub fn sign_checked_pczt<P: consensus::Parameters>(
/// Signs a checked, normalized PCZT and confirms in memory that every
/// supported shielded action owned by (`seed_fingerprint`, `account_index`)
/// received a spend authorization signature. Batch policy: additionally rejects
-/// PCZT shapes the batch flow does not support and requires at least one owned
+/// PCZT shapes the batch flow does not support, including Orchard or Ironwood
+/// spend authorization signatures, and requires at least one owned
/// signable shielded action. Parses `checked_pczt` exactly once and returns the
/// redacted, version-stamped response bytes. Derives keys into a fresh
/// [`SpendAuthCache`]; batch loops should use
@@ -2109,6 +2125,113 @@ mod tests {
);
}
+ fn assert_existing_batch_signature_error<T: core::fmt::Debug>(result: Result<T>, pool: &str) {
+ assert_eq!(
+ result.unwrap_err(),
+ ZcashError::InvalidPczt(alloc::format!(
+ "Zcash batch request must not contain {pool} spend authorization signatures"
+ ))
+ );
+ }
+
+ fn pczt_with_existing_funded_signature(
+ mut sample: pczt::test_support::SamplePczt,
+ pool: SignableShieldedPool,
+ ) -> pczt::test_support::SamplePczt {
+ use zcash_vendor::{
+ orchard::keys::{SpendAuthorizingKey, SpendingKey},
+ pczt::roles::signer::Signer,
+ zip32,
+ };
+
+ let original = Pczt::parse(&sample.bytes).expect("sample PCZT should parse");
+ let account = zip32::AccountId::ZERO;
+ let (required_actions, original) = match pool {
+ SignableShieldedPool::Orchard => signable_shielded_actions(
+ &MainNetwork,
+ original,
+ &sample.seed_fingerprint,
+ account,
+ ShieldedActionPolicy::Single,
+ ),
+ SignableShieldedPool::Ironwood => signable_shielded_actions(
+ &pczt::test_support::Nu6_3Network,
+ original,
+ &sample.seed_fingerprint,
+ account,
+ ShieldedActionPolicy::Single,
+ ),
+ }
+ .expect("sample PCZT should have a signable funded action");
+ assert_eq!(required_actions.len(), 1);
+
+ let spending_key = SpendingKey::from_zip32_seed(&sample.seed, 133, account).unwrap();
+ let ask = SpendAuthorizingKey::from(&spending_key);
+ let mut signer = Signer::new(original).expect("sample PCZT should be signable");
+ let action_index = required_actions[0].index;
+ let signing_result = match pool {
+ SignableShieldedPool::Orchard => signer.sign_orchard(action_index, &ask),
+ SignableShieldedPool::Ironwood => signer.sign_ironwood(action_index, &ask),
+ };
+ signing_result.expect("funded action should sign");
+ let pczt = signer.finish();
+ let actions = match pool {
+ SignableShieldedPool::Orchard => pczt.orchard().actions(),
+ SignableShieldedPool::Ironwood => pczt.ironwood().actions(),
+ };
+ assert_eq!(
+ actions
+ .iter()
+ .filter(|action| action.spend().spend_auth_sig().is_some())
+ .count(),
+ 1,
+ );
+ assert!(actions
+ .iter()
+ .any(|action| action.spend().spend_auth_sig().is_none()));
+ sample.bytes = pczt
+ .serialize()
+ .expect("partially signed PCZT should serialize");
+ sample
+ }
+
+ #[test]
+ fn test_batch_rejects_existing_funded_spend_auth_signature() {
+ for (sample, pool) in [
+ (
+ pczt::test_support::sample_orchard_change_pczt(),
+ SignableShieldedPool::Orchard,
+ ),
+ (
+ pczt::test_support::sample_ironwood_pczt(),
+ SignableShieldedPool::Ironwood,
+ ),
+ ] {
+ let sample = pczt_with_existing_funded_signature(sample, pool);
+
+ assert_existing_batch_signature_error(
+ check_batch_pczt_with_display(
+ &pczt::test_support::Nu6_3Network,
+ &sample.bytes,
+ &BatchCheckContext::new(&sample.ufvk_text),
+ &sample.seed_fingerprint,
+ 0,
+ ),
+ pool.label(),
+ );
+ assert_existing_batch_signature_error(
+ sign_checked_batch_pczt(
+ &pczt::test_support::Nu6_3Network,
+ &sample.bytes,
+ &sample.seed,
+ &sample.seed_fingerprint,
+ 0,
+ ),
+ pool.label(),
+ );
+ }
+ }
+
#[test]
fn test_sign_checked_batch_pczt_signs_ironwood_spend() {
let sample = pczt::test_support::sample_ironwood_pczt();
Why this scored 56/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.