feat: check pczt transparent inputs with multi-coins firmware
What the developer wrote
Authored by soralit
feat: check pczt transparent inputs with multi-coins firmware
What changed, and why it matters
This commit changes how the Keystone hardware wallet checks Zcash transparent inputs when using multi-coin firmware. Previously, Zcash transaction checking was completely disabled for SLIP39/passphrase wallets via a 'disabled' flag. The patch removes that blanket disable and instead adds a runtime check that at least one transparent input actually belongs to the wallet account being used. If none do, it returns a new 'PcztNoMyInputs' error. The change also slightly lowers code-coverage thresholds for the Zcash Rust module.
Treat as a security-hardening change that may fix or reduce a risk of signing a transaction that does not spend the user's own funds. Review the new check_sfp logic for correctness, ensure the lowered coverage thresholds are temporary and accompanied by tests for the new PcztNoMyInputs path, and verify that orchard/shielded inputs receive equivalent ownership checks if applicable.
Security signals we found
New validation rule: at least one transparent input must belong to the provided account in multi-coins mode
Removal of blanket disable for SLIP39/passphrase wallets in Zcash multi-coin checking
New error variant PcztNoMyInputs mapped to MasterFingerprintMismatch
Coverage thresholds lowered, suggesting new code paths are not fully tested
Evidence from the diff
The patch modifies rust/apps/zcash/src/pczt/check.rs so that check_pczt_transparent accepts a new boolean check_sfp. When true (multi-coins builds), check_transparent now tracks whether any input matched the account’s transparent pubkey and, if none matched, returns the new ZcashError::PcztNoMyInputs. The C/Rust FFI wrapper check_zcash_tx_multi_coins loses its disabled parameter, and gui_zcash.c stops passing mnemonicType == MNEMONIC_TYPE_SLIP39 as a disable switch. Coverage gates in the CI workflow are relaxed by 1 percentage point for regions and lines. Error mapping maps PcztNoMyInputs to MasterFingerprintMismatch.
Changed components
rust/apps/zcash/src/pczt/check.rsrust/apps/zcash/src/lib.rsrust/apps/zcash/src/errors.rsrust/rust_c/src/zcash/mod.rsrust/rust_c/src/common/errors.rssrc/ui/gui_chain/multi/gui_zcash.c.github/workflows/rust-zcash-checks.ymlInspect captured patch +33 / −17
diff --git a/.github/workflows/rust-zcash-checks.yml b/.github/workflows/rust-zcash-checks.yml
index 64ff85a..31f91ed 100644
--- a/.github/workflows/rust-zcash-checks.yml
+++ b/.github/workflows/rust-zcash-checks.yml
@@ -24,4 +24,4 @@ jobs:
uses: taiki-e/install-action@cargo-llvm-cov
- name: Run rust/apps/zcash
- run: cd rust/apps/zcash && cargo +nightly-2025-05-01 llvm-cov --fail-under-regions 70 --fail-under-functions 71 --fail-under-lines 77 --ignore-filename-regex 'keystore/*|utils/*|zcash_vendor/*'
+ run: cd rust/apps/zcash && cargo +nightly-2025-05-01 llvm-cov --fail-under-regions 69 --fail-under-functions 71 --fail-under-lines 76 --ignore-filename-regex 'keystore/*|utils/*|zcash_vendor/*'
diff --git a/rust/apps/zcash/src/errors.rs b/rust/apps/zcash/src/errors.rs
index c35071c..7e3c5f2 100644
--- a/rust/apps/zcash/src/errors.rs
+++ b/rust/apps/zcash/src/errors.rs
@@ -18,6 +18,8 @@ pub enum ZcashError {
SigningError(String),
#[error("invalid pczt, {0}")]
InvalidPczt(String),
+ #[error("None of inputs belong to the provided account")]
+ PcztNoMyInputs,
}
#[cfg(feature = "cypherpunk")]
diff --git a/rust/apps/zcash/src/lib.rs b/rust/apps/zcash/src/lib.rs
index c0c7a9d..616fafd 100644
--- a/rust/apps/zcash/src/lib.rs
+++ b/rust/apps/zcash/src/lib.rs
@@ -74,7 +74,7 @@ pub fn check_pczt_cypherpunk<P: consensus::Parameters>(
"transparent xpub is not present".to_string(),
))?;
pczt::check::check_pczt_orchard(params, seed_fingerprint, account_index, &ufvk, &pczt)?;
- pczt::check::check_pczt_transparent(params, seed_fingerprint, account_index, xpub, &pczt)
+ pczt::check::check_pczt_transparent(params, seed_fingerprint, account_index, xpub, &pczt, false)
}
#[cfg(feature = "multi_coins")]
@@ -115,6 +115,7 @@ pub fn check_pczt_multi_coins<P: consensus::Parameters>(
account_index,
&account_pubkey,
&pczt,
+ true,
)
}
diff --git a/rust/apps/zcash/src/pczt/check.rs b/rust/apps/zcash/src/pczt/check.rs
index 22b7a3c..2fc656f 100644
--- a/rust/apps/zcash/src/pczt/check.rs
+++ b/rust/apps/zcash/src/pczt/check.rs
@@ -42,13 +42,24 @@ pub fn check_pczt_transparent<P: consensus::Parameters>(
account_index: zip32::AccountId,
xpub: &AccountPubKey,
pczt: &Pczt,
+ check_sfp: bool,
) -> Result<(), ZcashError> {
Verifier::new(pczt.clone())
.with_transparent(|bundle| {
- check_transparent(params, seed_fingerprint, account_index, xpub, bundle)
- .map_err(pczt::roles::verifier::TransparentError::Custom)
+ check_transparent(
+ params,
+ seed_fingerprint,
+ account_index,
+ xpub,
+ bundle,
+ check_sfp,
+ )
+ .map_err(pczt::roles::verifier::TransparentError::Custom)
})
- .map_err(|e| ZcashError::InvalidDataError(alloc::format!("{e:?}")))?;
+ .map_err(|e| match e {
+ pczt::roles::verifier::TransparentError::Custom(e) => e,
+ _e => ZcashError::InvalidDataError(alloc::format!("{:?}", _e)),
+ })?;
Ok(())
}
@@ -58,15 +69,23 @@ fn check_transparent<P: consensus::Parameters>(
account_index: zip32::AccountId,
xpub: &AccountPubKey,
bundle: &transparent::pczt::Bundle,
+ check_sfp: bool,
) -> Result<(), ZcashError> {
+ let mut has_my_input = false;
bundle.inputs().iter().try_for_each(|input| {
- check_transparent_input(params, seed_fingerprint, account_index, xpub, input)?;
+ let _has = check_transparent_input(params, seed_fingerprint, account_index, xpub, input)?;
+ if _has {
+ has_my_input = true;
+ }
Ok::<_, ZcashError>(())
})?;
bundle.outputs().iter().try_for_each(|output| {
check_transparent_output(params, seed_fingerprint, account_index, xpub, output)?;
Ok::<_, ZcashError>(())
})?;
+ if check_sfp && !has_my_input {
+ return Err(ZcashError::PcztNoMyInputs);
+ }
Ok(())
}
@@ -76,7 +95,7 @@ fn check_transparent_input<P: consensus::Parameters>(
account_index: zip32::AccountId,
xpub: &AccountPubKey,
input: &transparent::pczt::Input,
-) -> Result<(), ZcashError> {
+) -> Result<bool, ZcashError> {
let script = input.script_pubkey().clone();
//p2sh transparent input is not supported yet
match script.address() {
@@ -89,7 +108,7 @@ fn check_transparent_input<P: consensus::Parameters>(
match my_derivation {
None => {
//not my input, pass
- Ok(())
+ Ok(false)
}
Some((pubkey, derivation)) => {
// 2: derive my pubkey
@@ -116,7 +135,7 @@ fn check_transparent_input<P: consensus::Parameters>(
"transparent input script pubkey mismatch".to_string(),
));
}
- Ok(())
+ Ok(true)
}
}
}
diff --git a/rust/rust_c/src/common/errors.rs b/rust/rust_c/src/common/errors.rs
index ec07dcf..63cd0e4 100644
--- a/rust/rust_c/src/common/errors.rs
+++ b/rust/rust_c/src/common/errors.rs
@@ -583,6 +583,7 @@ impl From<&ZcashError> for ErrorCodes {
ZcashError::InvalidDataError(_) => Self::InvalidData,
ZcashError::SigningError(_) => Self::ZcashSigningError,
ZcashError::InvalidPczt(_) => Self::ZcashInvalidPczt,
+ ZcashError::PcztNoMyInputs => Self::MasterFingerprintMismatch,
}
}
}
diff --git a/rust/rust_c/src/zcash/mod.rs b/rust/rust_c/src/zcash/mod.rs
index 530e46a..e775032 100644
--- a/rust/rust_c/src/zcash/mod.rs
+++ b/rust/rust_c/src/zcash/mod.rs
@@ -107,14 +107,7 @@ pub unsafe extern "C" fn check_zcash_tx_multi_coins(
xpub: PtrString,
seed_fingerprint: PtrBytes,
account_index: u32,
- disabled: bool,
) -> *mut TransactionCheckResult {
- if disabled {
- return TransactionCheckResult::from(RustCError::UnsupportedTransaction(
- "zcash is not supported for slip39 and passphrase wallet now".to_string(),
- ))
- .c_ptr();
- }
let pczt = extract_ptr_with_type!(tx, ZcashPczt);
let xpub_text = unsafe { recover_c_char(xpub) };
let seed_fingerprint = extract_array!(seed_fingerprint, u8, 32);
diff --git a/src/ui/gui_chain/multi/gui_zcash.c b/src/ui/gui_chain/multi/gui_zcash.c
index 122f0d8..21c45c6 100644
--- a/src/ui/gui_chain/multi/gui_zcash.c
+++ b/src/ui/gui_chain/multi/gui_zcash.c
@@ -313,7 +313,7 @@ PtrT_TransactionCheckResult GuiGetZcashCheckResult(void)
#ifdef WEB3_VERSION
char *xpub = GetCurrentAccountPublicKey(XPUB_TYPE_ZEC_TRANSPARENT_LEGACY);
- return check_zcash_tx_multi_coins(data, xpub, sfp, zcash_account_index, mnemonicType == MNEMONIC_TYPE_SLIP39);
+ return check_zcash_tx_multi_coins(data, xpub, sfp, zcash_account_index);
#endif
#ifdef CYPHERPUNK_VERSION
char ufvk[ZCASH_UFVK_MAX_LEN + 1] = {0};
Why this scored 42/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.