What changed, and why it matters
This commit fixes how the Keystone hardware wallet generates Monero 'key images' for transactions that use extra one-time public keys (subaddresses or additional transaction keys). Before the fix, the code always used the main transaction public key to derive key images, even when Monero rules say it should use an additional key. This mismatch could cause the wallet to generate the wrong key image, which in a hardware wallet context most likely means a transaction would fail to build or be rejected, rather than funds being stolen. The commit adds logic to pick the correct additional key based on subaddress major/minor indices and the output index.
Treat as a security-relevant correctness fix. Review whether any prior firmware version could produce or sign Monero transactions involving subaddresses with incorrect key images, and assess if this could be induced by a malicious host to leak information or cause denial of service. Issue a firmware update note for Monero users who rely on subaddresses.
Security signals we found
Cryptographic key derivation logic changed
Subaddress handling added for key image generation
Additional transaction public keys now consumed
Potential for invalid key images / transaction construction failure before fix
Evidence from the diff
The patch modifies try_to_generate_image in rust/apps/monero/src/key_images.rs and its caller in transfer.rs. It now accepts a Vec
Changed components
rust/apps/monero/src/key_images.rsrust/apps/monero/src/transfer.rsInspect captured patch +28 / −4
diff --git a/rust/apps/monero/src/key_images.rs b/rust/apps/monero/src/key_images.rs
index 27b39bb..7e08d87 100644
--- a/rust/apps/monero/src/key_images.rs
+++ b/rust/apps/monero/src/key_images.rs
@@ -202,20 +202,37 @@ pub fn try_to_generate_image(
keypair: &KeyPair,
tx_pubkey: &[u8; 32],
output_pubkey: &[u8; 32],
+ additional_tx_keys: Vec<PublicKey>,
internal_output_index: u64,
major: u32,
optional_minors: Vec<u32>,
) -> Result<(Keyimage, Scalar)> {
+ let mut additional_tx_pub_key = None;
+ if major != 0 || !optional_minors.is_empty() {
+ if additional_tx_keys.len() == 1 {
+ additional_tx_pub_key = Some(additional_tx_keys[0]);
+ } else if !additional_tx_keys.is_empty() {
+ if internal_output_index as usize >= additional_tx_keys.len() {
+ return Err(MoneroError::GenerateKeyImageError);
+ }
+ additional_tx_pub_key = Some(additional_tx_keys[internal_output_index as usize]);
+ }
+ }
+
+ let key_to_use = match additional_tx_pub_key {
+ Some(key) => &key.as_bytes(),
+ None => tx_pubkey,
+ };
+
for minor in optional_minors {
let offset =
- calc_output_key_offset(keypair, tx_pubkey, internal_output_index, major, minor);
- match generate_key_image_from_offset(
+ calc_output_key_offset(keypair, key_to_use, internal_output_index, major, minor);
+ if let Some(image) = generate_key_image_from_offset(
&keypair.spend,
&offset,
&PublicKey::from_bytes(output_pubkey).unwrap(),
) {
- Some(image) => return Ok((Keyimage::new(image.compress().to_bytes()), offset)),
- None => continue,
+ return Ok((Keyimage::new(image.compress().to_bytes()), offset));
};
}
diff --git a/rust/apps/monero/src/transfer.rs b/rust/apps/monero/src/transfer.rs
index ba89164..7f938be 100644
--- a/rust/apps/monero/src/transfer.rs
+++ b/rust/apps/monero/src/transfer.rs
@@ -343,10 +343,17 @@ impl TxConstructionData {
let output_entry = source.outputs[source.real_output as usize];
let ctkey = output_entry.key;
+ let additional_tx_keys: Vec<PublicKey> = source
+ .real_out_additional_tx_keys
+ .iter()
+ .map(|key_bytes| PublicKey::from_bytes(key_bytes).unwrap())
+ .collect();
+
match try_to_generate_image(
keypair,
&source.real_out_tx_key,
&ctkey.dest,
+ additional_tx_keys,
source.real_output_in_tx_index,
self.subaddr_account,
self.subaddr_indices.clone(),
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.