fix sign message utf-8 string check
What the developer wrote
Authored by ww3512687
fix sign message utf-8 string check
What changed, and why it matters
This commit fixes a crash in the Keystone hardware wallet firmware when scanning QR codes that contain non-UTF-8 bytes. Previously, the code would panic and abort processing. The fix replaces the strict conversion with a checked conversion that returns a controlled error instead of crashing. It also ensures that when a 'signmessage' QR code contains invalid UTF-8, the device reports an unsupported transaction rather than crashing.
Review whether `infer_qrcode_type` should also reject or handle invalid UTF-8 input rather than silently using a lossy string for protocol detection. Confirm that the returned error message does not leak sensitive memory. Consider fuzzing the QR parsing entry points with invalid UTF-8 and truncated C strings.
Security signals we found
Removal of `unwrap()` on user-controlled C string conversion
Addition of explicit UTF-8 validation before message parsing
Controlled error return instead of panic for malformed QR input
Potential DoS mitigation against crash-inducing QR payloads
Evidence from the diff
The change introduces check_recover_c_char_lossy, which attempts CStr::to_str() and, on failure, returns (false, lossy_string) instead of panicking via unwrap(). infer_qrcode_type now uses the lossy variant but ignores the validity flag, which is acceptable because it only checks a prefix. parse_qrcode_text uses the flag and returns RustCError::UnsupportedTransaction(value) if the input is not valid UTF-8. This removes a panic path and prevents a malformed QR code from causing a firmware crash/reboot during QR parsing.
Changed components
rust/rust_c/src/common/qrcode/mod.rsrust/rust_c/src/common/utils.rsQR code parsing / signmessage handling pathInspect captured patch +13 / −3
diff --git a/rust/rust_c/src/common/qrcode/mod.rs b/rust/rust_c/src/common/qrcode/mod.rs
index 8c0464b..8b1d3b7 100644
--- a/rust/rust_c/src/common/qrcode/mod.rs
+++ b/rust/rust_c/src/common/qrcode/mod.rs
@@ -7,7 +7,7 @@ use super::{
errors::RustCError,
types::{Ptr, PtrString},
ur::URParseResult,
- utils::recover_c_char,
+ utils::check_recover_c_char_lossy,
};
#[repr(C)]
@@ -18,7 +18,7 @@ pub enum QRProtocol {
#[no_mangle]
pub unsafe extern "C" fn infer_qrcode_type(qrcode: PtrString) -> QRProtocol {
- let value = recover_c_char(qrcode);
+ let (_, value) = check_recover_c_char_lossy(qrcode);
if value.to_uppercase().starts_with("UR:") {
QRProtocol::QRCodeTypeUR
} else {
@@ -28,7 +28,10 @@ pub unsafe extern "C" fn infer_qrcode_type(qrcode: PtrString) -> QRProtocol {
#[no_mangle]
pub unsafe extern "C" fn parse_qrcode_text(qr: PtrString) -> Ptr<URParseResult> {
- let value = recover_c_char(qr);
+ let (is_ok, value) = check_recover_c_char_lossy(qr);
+ if !is_ok {
+ return URParseResult::from(RustCError::UnsupportedTransaction(value)).c_ptr();
+ }
if value.to_lowercase().starts_with("signmessage") {
if let Some((headers, message)) = value.split_once(':') {
if message.is_empty() {
diff --git a/rust/rust_c/src/common/utils.rs b/rust/rust_c/src/common/utils.rs
index 9d5f711..22b8603 100644
--- a/rust/rust_c/src/common/utils.rs
+++ b/rust/rust_c/src/common/utils.rs
@@ -17,6 +17,13 @@ pub unsafe fn recover_c_char(s: *mut c_char) -> String {
CStr::from_ptr(s).to_str().unwrap().to_string()
}
+pub unsafe fn check_recover_c_char_lossy(s: *mut c_char) -> (bool, String) {
+ match CStr::from_ptr(s).to_str() {
+ Ok(value) => (true, value.to_string()),
+ Err(_) => (false, CStr::from_ptr(s).to_string_lossy().into_owned()),
+ }
+}
+
pub unsafe fn recover_c_array<'a, T: Free>(s: PtrT<CSliceFFI<T>>) -> &'a [T] {
let boxed_keys = extract_ptr_with_type!(s, CSliceFFI<T>);
extract_array!(boxed_keys.data, T, boxed_keys.size)
Why this scored 51/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.