What the developer wrote
Authored by ww3512687
fix zcash unwrap
What changed, and why it matters
This commit changes two Zcash-related functions in the Keystone hardware wallet firmware so that the wallet seed is no longer treated as a mutable buffer and is no longer explicitly wiped (zeroized) from memory after use. The title says 'fix zcash unwrap,' suggesting the change was made to resolve a Rust panic caused by calling .unwrap() or by a mutability mismatch. Removing zeroize reduces memory cleanup of the secret seed, which could leave sensitive material in memory longer than intended. However, the diff alone does not prove an exploitable vulnerability; it may simply trade one bug for another.
Review whether the zeroize removal is strictly necessary; if the borrow checker conflict can be resolved by collecting the seed into an owned, zeroizable Vec or by using a zeroizing wrapper, restore explicit cleanup. Also replace the .unwrap() in rust_derive_iv_from_seed with proper error handling that returns a SimpleResponse error to the C caller. Add unit tests and a security note explaining why zeroize was removed or replaced.
Security signals we found
Removal of explicit secret scrubbing (zeroize) on the wallet seed
Change from mutable to immutable extraction of seed bytes
C FFI boundary handling highly sensitive material (master seed / private key derivation)
Presence of .unwrap() on a private-key derivation result in rust_derive_iv_from_seed
Small, single-file patch with no tests or documentation explaining security rationale
Evidence from the diff
In rust/rust_c/src/zcash/mod.rs, both derive_zcash_ufvk and rust_derive_iv_from_seed switch from extract_array_mut! to extract_array! and remove the subsequent seed.zeroize() call. The likely intent is to fix a compilation/runtime error where an immutable borrow from extract_array! conflicted with the mutability required by zeroize(), or where an unwrap() in the call chain panicked. By dropping mutability and zeroize, the seed bytes are no longer cleared after use. Because these are C FFI functions handling the master seed, any reduction in explicit cleanup increases the window for memory disclosure, though the actual risk depends on allocator behavior, stack/register clearing, and whether the seed is copied elsewhere.
Changed components
rust/rust_c/src/zcash/mod.rsderive_zcash_ufvk FFI functionrust_derive_iv_from_seed FFI functionZcash unified full viewing key derivationZcash IV/private key derivation from seedInspect captured patch +2 / −4
diff --git a/rust/rust_c/src/zcash/mod.rs b/rust/rust_c/src/zcash/mod.rs
index 48cef70..f488b0f 100644
--- a/rust/rust_c/src/zcash/mod.rs
+++ b/rust/rust_c/src/zcash/mod.rs
@@ -30,14 +30,13 @@ pub unsafe extern "C" fn derive_zcash_ufvk(
seed_len: u32,
account_path: PtrString,
) -> *mut SimpleResponse<c_char> {
- let mut seed = extract_array_mut!(seed, u8, seed_len as usize);
+ let seed = extract_array!(seed, u8, seed_len as usize);
let account_path = unsafe { recover_c_char(account_path) };
let ufvk_text = derive_ufvk(&MainNetwork, seed, &account_path);
let result = match ufvk_text {
Ok(text) => SimpleResponse::success(convert_c_char(text)).simple_c_ptr(),
Err(e) => SimpleResponse::from(e).simple_c_ptr(),
};
- seed.zeroize();
result
}
@@ -196,12 +195,11 @@ pub unsafe extern "C" fn rust_derive_iv_from_seed(
seed: PtrBytes,
seed_len: u32,
) -> *mut SimpleResponse<u8> {
- let mut seed = extract_array_mut!(seed, u8, seed_len as usize);
+ let seed = extract_array!(seed, u8, seed_len as usize);
let iv_path = "m/44'/1557192335'/0'/2'/0'".to_string();
let iv = get_private_key_by_seed(seed, &iv_path).unwrap();
let mut iv_bytes = [0; 16];
iv_bytes.copy_from_slice(&iv[..16]);
- seed.zeroize();
SimpleResponse::success(Box::into_raw(Box::new(iv_bytes)) as *mut u8).simple_c_ptr()
}
Why this scored 57/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.