What the developer wrote
Authored by ww3512687
seed clear
What changed, and why it matters
This commit updates Ethereum signing code in a cryptocurrency hardware wallet firmware so that the secret seed (master key) is wiped from memory immediately after use. It also replaces several 'unwrap' crash points with proper error handling. The main security improvement is reducing the window where the seed sits in memory after signing, which could lower the risk of an attacker reading it from device RAM. However, the patch is partial: it only covers three Ethereum signing functions and does not appear to add zeroize to all signing paths in the firmware.
Review whether extract_array_mut! and seed.zeroize() are applied consistently across all signing modules (Bitcoin, Cosmos, Solana, etc.) and all seed entry points. Verify that the C/Rust boundary does not leave a second copy of the seed in memory. Consider adding a secure memset wrapper and auditing for other .unwrap() calls in security-critical paths.
Security signals we found
Secret (BIP39 seed) zeroization after cryptographic use
Reduction of .unwrap() panic surfaces in signing path
Memory-safety hardening in hardware-wallet firmware
Partial coverage: only Ethereum signing functions modified
Evidence from the diff
The diff modifies rust/rust_c/src/ethereum/mod.rs. It imports zeroize::Zeroize and a new extract_array_mut! macro, changes seed slices to mutable in eth_sign_batch_tx, eth_sign_tx_dynamic, and eth_sign_tx_bytes, and calls seed.zeroize() after signing. It also replaces multiple .unwrap() calls in eth_sign_tx_bytes with match-based error returns. The change is defensive: it clears the BIP39 seed buffer after use and hardens error handling. It does not, however, demonstrate that all seed-handling call sites across the codebase are zeroized, nor does it change how the seed is originally copied into Rust memory.
Changed components
rust/rust_c/src/ethereum/mod.rseth_sign_batch_txeth_sign_tx_dynamiceth_sign_tx_bytesInspect captured patch +52 / −15
diff --git a/rust/rust_c/src/ethereum/mod.rs b/rust/rust_c/src/ethereum/mod.rs
index 6a34329..b808ef6 100644
--- a/rust/rust_c/src/ethereum/mod.rs
+++ b/rust/rust_c/src/ethereum/mod.rs
@@ -32,12 +32,12 @@ use crate::common::ur::{
};
use crate::common::utils::{convert_c_char, recover_c_char};
use crate::common::KEYSTONE;
-use crate::{extract_array, extract_ptr_with_type};
-
+use crate::{extract_array, extract_array_mut, extract_ptr_with_type};
use structs::{
DisplayETH, DisplayETHBatchTx, DisplayETHPersonalMessage, DisplayETHTypedData,
EthParsedErc20Approval, EthParsedErc20Transaction, TransactionType,
};
+use zeroize::Zeroize;
mod abi;
pub mod address;
@@ -450,7 +450,7 @@ pub unsafe extern "C" fn eth_sign_batch_tx(
seed_len: u32,
) -> PtrT<UREncodeResult> {
let batch_transaction = extract_ptr_with_type!(ptr, EthBatchSignRequest);
- let seed = extract_array!(seed, u8, seed_len as usize);
+ let mut seed = extract_array_mut!(seed, u8, seed_len as usize);
let mut result = Vec::new();
for request in batch_transaction.get_requests() {
let mut path = match request.get_derivation_path().get_path() {
@@ -483,6 +483,8 @@ pub unsafe extern "C" fn eth_sign_batch_tx(
.c_ptr()
}
};
+
+ seed.zeroize();
match signature {
Err(e) => return UREncodeResult::from(e).c_ptr(),
Ok(sig) => {
@@ -546,7 +548,7 @@ pub unsafe extern "C" fn eth_sign_tx_dynamic(
fragment_length: usize,
) -> PtrT<UREncodeResult> {
let crypto_eth = extract_ptr_with_type!(ptr, EthSignRequest);
- let seed = extract_array!(seed, u8, seed_len as usize);
+ let mut seed = extract_array_mut!(seed, u8, seed_len as usize);
let mut path = match crypto_eth.get_derivation_path().get_path() {
Some(v) => v,
None => return UREncodeResult::from(EthereumError::InvalidTransaction).c_ptr(),
@@ -577,6 +579,7 @@ pub unsafe extern "C" fn eth_sign_tx_dynamic(
app_ethereum::sign_typed_data_message(&sign_data, seed, &path)
}
};
+ seed.zeroize();
match signature {
Err(e) => UREncodeResult::from(e).c_ptr(),
Ok(sig) => {
@@ -612,9 +615,8 @@ pub unsafe extern "C" fn eth_sign_tx_bytes(
return UREncodeResult::from(KeystoneError::ProtobufError(e.to_string())).c_ptr();
}
};
- let tx = sign_tx.transaction.unwrap();
- let eth_tx = match tx {
- EthTx(tx) => tx,
+ let eth_tx = match sign_tx.transaction {
+ Some(EthTx(tx)) => tx,
_ => {
return UREncodeResult::from(RustCError::InvalidData(
"Cant get eth tx struct data".to_string(),
@@ -623,15 +625,38 @@ pub unsafe extern "C" fn eth_sign_tx_bytes(
}
};
- let legacy_transaction = LegacyTransaction::try_from(eth_tx).unwrap();
+ let legacy_transaction = match LegacyTransaction::try_from(eth_tx) {
+ Ok(tx) => tx,
+ Err(_) => {
+ return UREncodeResult::from(RustCError::InvalidData("invalid eth tx".to_string()))
+ .c_ptr();
+ }
+ };
- let seed = extract_array!(seed, u8, seed_len as usize);
+ let mut seed = extract_array_mut!(seed, u8, seed_len as usize);
let mfp = extract_array!(mfp, u8, mfp_len as usize);
- let signature =
- app_ethereum::sign_legacy_tx_v2(&legacy_transaction.encode_raw(), seed, &sign_tx.hd_path)
- .unwrap();
- let transaction_signature = TransactionSignature::try_from(signature).unwrap();
+ let signature = match app_ethereum::sign_legacy_tx_v2(
+ &legacy_transaction.encode_raw(),
+ seed,
+ &sign_tx.hd_path,
+ ) {
+ Ok(sig) => sig,
+ Err(e) => {
+ seed.zeroize();
+ return UREncodeResult::from(e).c_ptr();
+ }
+ };
+ seed.zeroize();
+ let transaction_signature = match TransactionSignature::try_from(signature) {
+ Ok(sig) => sig,
+ Err(_) => {
+ return UREncodeResult::from(RustCError::InvalidData(
+ "invalid transaction signature".to_string(),
+ ))
+ .c_ptr();
+ }
+ };
let legacy_tx_with_signature = legacy_transaction.set_signature(transaction_signature);
// tx_id is transaction hash , you can use this hash to search tx detail on the etherscan.
@@ -660,10 +685,22 @@ pub unsafe extern "C" fn eth_sign_tx_bytes(
};
let base_vec = ur_registry::pb::protobuf_parser::serialize_protobuf(base);
// zip data can reduce the size of the data
- let zip_data = pb::protobuf_parser::zip(&base_vec).unwrap();
+ let zip_data = match pb::protobuf_parser::zip(&base_vec) {
+ Ok(data) => data,
+ Err(e) => {
+ return UREncodeResult::from(RustCError::InvalidData(e.to_string())).c_ptr();
+ }
+ };
// data --> protobuf --> zip protobuf data --> cbor bytes data
+ let bytes = match ur_registry::bytes::Bytes::new(zip_data).try_into() {
+ Ok(b) => b,
+ Err(e) => {
+ return UREncodeResult::from(RustCError::InvalidData("invalid bytes".to_string()))
+ .c_ptr();
+ }
+ };
UREncodeResult::encode(
- ur_registry::bytes::Bytes::new(zip_data).try_into().unwrap(),
+ bytes,
ur_registry::bytes::Bytes::get_registry_type().get_type(),
FRAGMENT_MAX_LENGTH_DEFAULT,
)
Why this scored 59/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.