fix(zcash): bound batch count before parsing
What changed, and why it matters
This commit adds a safety check for Zcash batch transactions on Keystone hardware wallets. Before fully parsing a batch of PCZT (Zcash transaction) data, the firmware now reads the declared number of items and rejects it if it exceeds 50. This prevents the parser from trying to allocate memory for an absurdly large number of items based solely on attacker-controlled input, which could cause memory exhaustion or a crash.
Review whether `ZCASH_BATCH_MAX_PCZTS = 50` aligns with realistic device memory limits and expected user workflows. Audit `BatchSignRequest::parse()` for other unbounded allocations. Ensure `postcard` is pinned to a reviewed version and that the `usize` deserialization cannot itself be abused (e.g., via multibyte varint edge cases). Consider fuzzing the batch request parser.
Security signals we found
Pre-allocation input validation: bounds a length field before parser allocation
Potential denial-of-service vector mitigated: oversized count could exhaust device memory
New dependency `postcard` introduced for controlled length decoding
Magic/header and version check before interpreting length field
Unit test added to verify count enforcement before full parse
Evidence from the diff
The patch introduces validate_zcash_batch_request_count() in rust/rust_c/src/zcash/mod.rs, gated by the cypherpunk feature. It inspects the first 12 bytes of a batch request, verifies the magic/header and PCZT version (1 or 2), then uses postcard::take_from_bytes::<usize> to decode the sequence length before handing the buffer to BatchSignRequest::parse(). If the count exceeds ZCASH_BATCH_MAX_PCZTS (50), it returns RustCError::UnsupportedTransaction. The postcard dependency is added to rust_c/Cargo.toml and rust/Cargo.lock. A unit test confirms the limit is enforced before full parsing.
Changed components
rust/rust_c/src/zcash/mod.rsrust/rust_c/Cargo.tomlrust/Cargo.lockZcash batch signing (cypherpunk feature)Inspect captured patch +60 / −1
diff --git a/rust/Cargo.lock b/rust/Cargo.lock
index 8280a18..e21a0ec 100644
--- a/rust/Cargo.lock
+++ b/rust/Cargo.lock
@@ -3881,6 +3881,7 @@ dependencies = [
"keystore",
"minicbor",
"no_std_io2",
+ "postcard",
"rand_core 0.6.4",
"rsa",
"rust_tools",
diff --git a/rust/rust_c/Cargo.toml b/rust/rust_c/Cargo.toml
index f4259af..98a54e0 100644
--- a/rust/rust_c/Cargo.toml
+++ b/rust/rust_c/Cargo.toml
@@ -26,6 +26,7 @@ aes = { workspace = true }
cbc = { workspace = true }
cipher = { workspace = true }
minicbor = { workspace = true }
+postcard = { version = "1.0.3", default-features = false, optional = true }
#keystone owned
sim_qr_reader = { workspace = true, optional = true }
keystore = { workspace = true, default-features = false }
@@ -83,7 +84,7 @@ sui = ["dep:app_sui"]
ton = ["dep:app_ton"]
tron = ["dep:app_tron"]
xrp = ["dep:app_xrp"]
-zcash = ["dep:app_zcash", "dep:zcash_vendor"]
+zcash = ["dep:app_zcash", "dep:postcard", "dep:zcash_vendor"]
zcash_multi_coins = ["zcash", "app_zcash/multi_coins"]
zcash_cypherpunk = ["zcash", "app_zcash/cypherpunk"]
monero = ["dep:app_monero"]
diff --git a/rust/rust_c/src/zcash/mod.rs b/rust/rust_c/src/zcash/mod.rs
index 703f218..c7eabf2 100644
--- a/rust/rust_c/src/zcash/mod.rs
+++ b/rust/rust_c/src/zcash/mod.rs
@@ -37,6 +37,8 @@ use zeroize::Zeroize;
const ZCASH_BATCH_MAX_PCZTS: usize = 50;
#[cfg(feature = "cypherpunk")]
const ZCASH_BATCH_MAX_TOTAL_BYTES: usize = 512 * 1024;
+#[cfg(feature = "cypherpunk")]
+const ZCASH_BATCH_REQUEST_HEADER_LEN: usize = 12;
#[no_mangle]
pub unsafe extern "C" fn derive_zcash_ufvk(
@@ -284,10 +286,41 @@ fn validate_zcash_batch_envelope(request_id: &[u8], data: &[u8]) -> Result<(), R
Ok(())
}
+/// Rejects an oversized top-level count before Postcard allocates the PCZT vector.
+#[cfg(feature = "cypherpunk")]
+fn validate_zcash_batch_request_count(data: &[u8]) -> Result<(), RustCError> {
+ let Some(header) = data.get(..ZCASH_BATCH_REQUEST_HEADER_LEN) else {
+ return Ok(());
+ };
+
+ // Leave malformed or unknown headers to the canonical parser so it retains
+ // its existing error. The pinned parser recognizes PCZT versions 1 and 2.
+ let pczt_version = u32::from_le_bytes(header[8..12].try_into().unwrap());
+ if &header[..8] != b"PCZB\x01\0\0\0" || !matches!(pczt_version, 1 | 2) {
+ return Ok(());
+ }
+
+ // Decode only the sequence length; malformed bodies remain the canonical
+ // parser's responsibility.
+ let Ok((pczt_count, _)) =
+ postcard::take_from_bytes::<usize>(&data[ZCASH_BATCH_REQUEST_HEADER_LEN..])
+ else {
+ return Ok(());
+ };
+ if pczt_count > ZCASH_BATCH_MAX_PCZTS {
+ return Err(RustCError::UnsupportedTransaction(format!(
+ "Zcash batch supports at most {ZCASH_BATCH_MAX_PCZTS} PCZTs"
+ )));
+ }
+
+ Ok(())
+}
+
/// Parses the bounded outer registry into the PCZT crate's batch request.
#[cfg(feature = "cypherpunk")]
fn parse_zcash_batch_registry(registry: &ZcashSignBatch) -> Result<BatchSignRequest, RustCError> {
validate_zcash_batch_envelope(registry.get_request_id(), registry.get_data())?;
+ validate_zcash_batch_request_count(registry.get_data())?;
BatchSignRequest::parse(registry.get_data())
.map_err(|e| RustCError::InvalidData(format!("invalid PCZT batch request: {e:?}")))
}
@@ -970,6 +1003,30 @@ mod tests {
));
}
+ #[cfg(feature = "cypherpunk")]
+ #[test]
+ fn test_zcash_batch_rejects_count_before_parse() {
+ let mut request = empty_batch_request();
+ request[ZCASH_BATCH_REQUEST_HEADER_LEN] = ZCASH_BATCH_MAX_PCZTS as u8;
+ validate_zcash_batch_request_count(&request).unwrap();
+
+ let mut overlong_small_count = empty_batch_request();
+ overlong_small_count[ZCASH_BATCH_REQUEST_HEADER_LEN] = 0x81;
+ overlong_small_count.push(0);
+ validate_zcash_batch_request_count(&overlong_small_count).unwrap();
+
+ // The body declares 51 PCZTs but omits them. Reaching the count error
+ // proves the limit is enforced before the full request is parsed.
+ request[ZCASH_BATCH_REQUEST_HEADER_LEN] += 1;
+ let registry = ZcashSignBatch::new(vec![0xaa], request);
+ assert_eq!(
+ parse_zcash_batch_registry(®istry).unwrap_err(),
+ RustCError::UnsupportedTransaction(format!(
+ "Zcash batch supports at most {ZCASH_BATCH_MAX_PCZTS} PCZTs"
+ ))
+ );
+ }
+
#[cfg(feature = "cypherpunk")]
#[test]
fn test_encode_zcash_batch_sig_result_wraps_pczt_response() {
Why this scored 61/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.