fix the edge cases for stellar app
What the developer wrote
Authored by aaron
fix the edge cases for stellar app
What changed, and why it matters
This commit fixes several edge cases in the Stellar cryptocurrency app of the Keystone 3 hardware wallet. Previously, the code could crash or behave unpredictably when given malformed signing requests, such as data shorter than expected or missing wallet key paths. The patch adds length checks and proper error handling so the device returns controlled errors instead of panicking or reading invalid memory.
Treat this as a security hardening fix and include it in the next firmware release. Review other blockchain apps in the repository for similar missing length checks and unwrap() usage on attacker-influenced inputs. No independent CVE or advisory is referenced, so additional coordinated disclosure review is not required unless further issues are found.
Security signals we found
Addition of minimum-length check before slicing signature base (32-byte network ID prefix)
Replacement of unwrap() calls with explicit error propagation in build_signature_data and derivation path extraction
Length validation for TransactionHash sign data (exactly 32 bytes) in both parse and sign paths
Conversion of infallible/panicking APIs to Result-returning APIs in the Stellar app library
FFI boundary now returns error result objects instead of panicking on invalid input
Evidence from the diff
The patch hardens Stellar transaction parsing and signing in the Keystone 3 firmware. In rust/apps/stellar/src/lib.rs, get_network_from_base and strip_network_prefix now require at least 32 bytes before slicing, returning a Result instead of panicking on short inputs. In rust/rust_c/src/stellar/mod.rs, the FFI entry points stellar_parse and stellar_sign now validate sign_data length (32 bytes for TransactionHash, at least 32 bytes indirectly for Transaction via base_to_xdr), handle missing derivation paths, and propagate errors from StellarSignature::try_into instead of calling unwrap. These changes prevent panics and potential out-of-bounds reads when processing malicious or malformed Stellar sign requests.
Changed components
rust/apps/stellar/src/lib.rsrust/rust_c/src/stellar/mod.rsStellar transaction parsing flowStellar transaction signing flowStellarSignRequest FFI handlersInspect captured patch +50 / −19
diff --git a/rust/apps/stellar/src/lib.rs b/rust/apps/stellar/src/lib.rs
index 1cf0c9d..be57621 100644
--- a/rust/apps/stellar/src/lib.rs
+++ b/rust/apps/stellar/src/lib.rs
@@ -9,22 +9,34 @@ pub mod structs;
#[macro_use]
extern crate alloc;
+use crate::errors::{Result, StellarError};
use crate::structs::Network;
use alloc::string::String;
use alloc::vec::Vec;
-pub fn get_network_from_base(base: &[u8]) -> Network {
+fn ensure_signature_base_len(base: &[u8]) -> Result<()> {
+ if base.len() < 32 {
+ return Err(StellarError::InvalidData(
+ "signature base is shorter than 32 bytes".into(),
+ ));
+ }
+ Ok(())
+}
+
+pub fn get_network_from_base(base: &[u8]) -> Result<Network> {
+ ensure_signature_base_len(base)?;
let network_id = &base[0..32];
- Network::from_hash(network_id)
+ Ok(Network::from_hash(network_id))
}
-fn strip_network_prefix(base: &[u8]) -> Vec<u8> {
- base[32..].to_vec()
+fn strip_network_prefix(base: &[u8]) -> Result<Vec<u8>> {
+ ensure_signature_base_len(base)?;
+ Ok(base[32..].to_vec())
}
-pub fn base_to_xdr(base: &[u8]) -> String {
- let stripped_base = strip_network_prefix(base);
- base64::encode(&stripped_base)
+pub fn base_to_xdr(base: &[u8]) -> Result<String> {
+ let stripped_base = strip_network_prefix(base)?;
+ Ok(base64::encode(&stripped_base))
}
#[cfg(test)]
@@ -55,7 +67,7 @@ mod tests {
#[test]
fn test_hash_from() {
let signature_base = "7ac33997544e3175d266bd022439b22cdb16508c01163f26e5cb2a3e1045a979000000020000000096e8c54780e871fabf106cb5b047149e72b04aa5e069a158b2d0e7a68ab50d4f00002710031494870000000a00000001000000000000000000000000664ed303000000000000000100000000000000060000000155534443000000003b9911380efe988ba0a8900eb1cfe44f366f7dbe946bed077240f7f624df15c57fffffffffffffff00000000";
- let network = get_network_from_base(&hex::decode(signature_base).unwrap());
+ let network = get_network_from_base(&hex::decode(signature_base).unwrap()).unwrap();
assert_eq!(
network.get(),
"Public Global Stellar Network ; September 2015"
@@ -65,7 +77,7 @@ mod tests {
#[test]
fn test_strip_network_prefix() {
let signature_base = "7ac33997544e3175d266bd022439b22cdb16508c01163f26e5cb2a3e1045a979000000020000000096e8c54780e871fabf106cb5b047149e72b04aa5e069a158b2d0e7a68ab50d4f00002710031494870000000a00000001000000000000000000000000664ed303000000000000000100000000000000060000000155534443000000003b9911380efe988ba0a8900eb1cfe44f366f7dbe946bed077240f7f624df15c57fffffffffffffff00000000";
- let data = strip_network_prefix(&hex::decode(signature_base).unwrap());
+ let data = strip_network_prefix(&hex::decode(signature_base).unwrap()).unwrap();
assert_eq!(data.len(), 144);
assert_eq!(hex::encode(data), "000000020000000096e8c54780e871fabf106cb5b047149e72b04aa5e069a158b2d0e7a68ab50d4f00002710031494870000000a00000001000000000000000000000000664ed303000000000000000100000000000000060000000155534443000000003b9911380efe988ba0a8900eb1cfe44f366f7dbe946bed077240f7f624df15c57fffffffffffffff00000000");
}
diff --git a/rust/rust_c/src/stellar/mod.rs b/rust/rust_c/src/stellar/mod.rs
index 451aaac..7c7d629 100644
--- a/rust/rust_c/src/stellar/mod.rs
+++ b/rust/rust_c/src/stellar/mod.rs
@@ -37,8 +37,17 @@ pub unsafe extern "C" fn stellar_parse(
) -> PtrT<TransactionParseResult<DisplayStellarTx>> {
let sign_request = extract_ptr_with_type!(ptr, StellarSignRequest);
let raw_message = match sign_request.get_sign_type() {
- SignType::Transaction => base_to_xdr(&sign_request.get_sign_data()),
- SignType::TransactionHash => hex::encode(sign_request.get_sign_data()),
+ SignType::Transaction => match base_to_xdr(&sign_request.get_sign_data()) {
+ Ok(xdr) => xdr,
+ Err(e) => return TransactionParseResult::from(e).c_ptr(),
+ },
+ SignType::TransactionHash => {
+ let sign_data = sign_request.get_sign_data();
+ if sign_data.len() != 32 {
+ return TransactionParseResult::from(RustCError::InvalidData).c_ptr();
+ }
+ hex::encode(sign_data)
+ }
_ => {
return TransactionParseResult::from(RustCError::UnsupportedTransaction(
"Transaction".to_string(),
@@ -83,9 +92,11 @@ fn build_signature_data(
signature: &[u8],
sign_request: StellarSignRequest,
) -> PtrT<UREncodeResult> {
- let data = StellarSignature::new(sign_request.get_request_id(), signature.to_vec())
- .try_into()
- .unwrap();
+ let data =
+ match StellarSignature::new(sign_request.get_request_id(), signature.to_vec()).try_into() {
+ Ok(data) => data,
+ Err(e) => return UREncodeResult::from(e).c_ptr(),
+ };
UREncodeResult::encode(
data,
StellarSignature::get_registry_type().get_type(),
@@ -103,16 +114,24 @@ pub unsafe extern "C" fn stellar_sign(
let seed = extract_array!(seed, u8, seed_len as usize);
let sign_request = extract_ptr_with_type!(ptr, StellarSignRequest);
let sign_data = sign_request.get_sign_data();
- let path = sign_request.get_derivation_path().get_path().unwrap();
+ let path = match sign_request.get_derivation_path().get_path() {
+ Some(path) => path,
+ None => return UREncodeResult::from(RustCError::InvalidHDPath).c_ptr(),
+ };
match sign_request.get_sign_type() {
SignType::Transaction => match sign_signature_base(&sign_data, seed, &path) {
Ok(signature) => build_signature_data(&signature, sign_request.to_owned()),
Err(e) => UREncodeResult::from(e).c_ptr(),
},
- SignType::TransactionHash => match sign_hash(&sign_data, seed, &path) {
- Ok(signature) => build_signature_data(&signature, sign_request.to_owned()),
- Err(e) => UREncodeResult::from(e).c_ptr(),
- },
+ SignType::TransactionHash => {
+ if sign_data.len() != 32 {
+ return UREncodeResult::from(RustCError::InvalidData).c_ptr();
+ }
+ match sign_hash(&sign_data, seed, &path) {
+ Ok(signature) => build_signature_data(&signature, sign_request.to_owned()),
+ Err(e) => UREncodeResult::from(e).c_ptr(),
+ }
+ }
_ => UREncodeResult::from(RustCError::UnsupportedTransaction(
"Transaction".to_string(),
))
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.