From 2def3ef49e53f1dd243c2708fc77b676a17570b8 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Thu, 16 Jul 2026 15:42:55 +0200 Subject: [PATCH] =?UTF-8?q?refactor(host):=20extract=20native=5Fpairing/sa?= =?UTF-8?q?nitize.rs=20=E2=80=94=20the=20untrusted-name=20scrubber?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per plan §W5: move sanitize_device_name (+ its NAME_MAX cap and unit test) out of the native_pairing facade into native_pairing/sanitize.rs. It is a self-contained, security-relevant leaf — the one place a wire-supplied unpaired-device name is scrubbed of control chars / bidi-override spoofing before it is stored, listed, logged, or shown in the approval UI. Re-export via `pub(crate) use` so crate::native_pairing::sanitize_device_name stays stable (punktfunk1 accept loop + the two in-crate callers). Pure code-move; verified host clippy 0/0 + 11 native_pairing tests green on Linux (home-worker-5). Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/punktfunk-host/src/native_pairing.rs | 64 ++--------------- .../src/native_pairing/sanitize.rs | 69 +++++++++++++++++++ 2 files changed, 74 insertions(+), 59 deletions(-) create mode 100644 crates/punktfunk-host/src/native_pairing/sanitize.rs diff --git a/crates/punktfunk-host/src/native_pairing.rs b/crates/punktfunk-host/src/native_pairing.rs index 765be346..e20525e2 100644 --- a/crates/punktfunk-host/src/native_pairing.rs +++ b/crates/punktfunk-host/src/native_pairing.rs @@ -196,47 +196,11 @@ fn random_pin() -> String { format!("{:04}", rand::thread_rng().gen_range(0..10_000u32)) } -/// Sanitize a client-supplied device name before it's stored, listed, or logged. The name comes -/// straight off the wire (the `Hello`/`PairRequest` of an *unpaired* device), so it's untrusted: a -/// hostile LAN device could embed terminal escapes / control characters (log + console injection) or -/// bidi overrides (`U+202E` etc.) to make a malicious device *look* like a trusted one in the -/// approval UI. Strip C0/C1 controls and Unicode bidi/format controls, collapse whitespace, trim, and -/// cap the length; an empty/all-control name falls back to a fingerprint-derived label. -pub(crate) fn sanitize_device_name(name: &str, fp_hex: &str) -> String { - let cleaned: String = name - .chars() - .map(|c| if c == '\t' || c == '\n' { ' ' } else { c }) - .filter(|&c| { - !c.is_control() - // Bidi/format controls that could spoof or reorder the displayed name. - && !('\u{202A}'..='\u{202E}').contains(&c) // LRE..RLO/PDF - && !('\u{2066}'..='\u{2069}').contains(&c) // LRI..PDI - && c != '\u{200E}' // LRM - && c != '\u{200F}' // RLM - && c != '\u{061C}' // ALM - && c != '\u{FEFF}' // BOM / zero-width no-break space - }) - .collect(); - // Collapse internal whitespace runs, trim, cap at the wire limit. - let collapsed = cleaned.split_whitespace().collect::>().join(" "); - let mut trimmed = collapsed.as_str(); - while trimmed.len() > NAME_MAX { - let mut cut = NAME_MAX; - while !trimmed.is_char_boundary(cut) { - cut -= 1; - } - trimmed = &trimmed[..cut]; - } - let trimmed = trimmed.trim(); - if trimmed.is_empty() { - format!("device {}", &fp_hex[..8.min(fp_hex.len())]) - } else { - trimmed.to_string() - } -} - -/// Max stored device-name length (matches the `Hello` wire cap, `quic::HELLO_NAME_MAX`). -const NAME_MAX: usize = 64; +/// The untrusted-device-name sanitizer lives in its own module (plan §W5); re-exported so +/// `crate::native_pairing::sanitize_device_name` stays stable (the `punktfunk1` accept loop +/// reaches it there). +mod sanitize; +pub(crate) use sanitize::sanitize_device_name; impl NativePairing { /// Load the trust store. `store_path = None` uses the default config path. If `arm_at_start` @@ -816,24 +780,6 @@ mod tests { let _ = std::fs::remove_file(&p); } - #[test] - fn sanitize_strips_control_and_bidi() { - // ANSI escape + newline + a bidi override that could spoof the displayed name. - let dirty = "\u{1b}]0;evil\u{07}Good\nDevice\u{202E}xfp"; - let clean = sanitize_device_name(dirty, "deadbeef00"); - assert!(!clean.contains('\u{1b}') && !clean.contains('\n') && !clean.contains('\u{202E}')); - // ESC dropped (']' survives), BEL dropped, '\n'→space (Good Device), RLO dropped (no space). - assert_eq!(clean, "]0;evilGood Devicexfp"); - // All-control / empty → fingerprint-derived fallback. - assert_eq!( - sanitize_device_name("\u{1b}\u{07}", "deadbeef00"), - "device deadbeef" - ); - assert_eq!(sanitize_device_name(" ", "abc"), "device abc"); - // Over-long names cap at a char boundary. - assert!(sanitize_device_name(&"x".repeat(200), "ab").len() <= 64); - } - #[test] fn pairing_clears_a_pending_knock() { let p = temp(); diff --git a/crates/punktfunk-host/src/native_pairing/sanitize.rs b/crates/punktfunk-host/src/native_pairing/sanitize.rs new file mode 100644 index 00000000..83413ead --- /dev/null +++ b/crates/punktfunk-host/src/native_pairing/sanitize.rs @@ -0,0 +1,69 @@ +//! Sanitize a client-supplied device name before it is stored, listed, logged, or shown in the +//! pairing-approval UI. The name arrives on the wire from an *unpaired* device, so it is untrusted +//! (terminal-escape / control-char injection, bidi-override spoofing of a trusted-looking name) — +//! this is the one place that scrubs it. Split out of the `native_pairing` facade (plan §W5). + +/// Sanitize a client-supplied device name before it's stored, listed, or logged. The name comes +/// straight off the wire (the `Hello`/`PairRequest` of an *unpaired* device), so it's untrusted: a +/// hostile LAN device could embed terminal escapes / control characters (log + console injection) or +/// bidi overrides (`U+202E` etc.) to make a malicious device *look* like a trusted one in the +/// approval UI. Strip C0/C1 controls and Unicode bidi/format controls, collapse whitespace, trim, and +/// cap the length; an empty/all-control name falls back to a fingerprint-derived label. +pub(crate) fn sanitize_device_name(name: &str, fp_hex: &str) -> String { + let cleaned: String = name + .chars() + .map(|c| if c == '\t' || c == '\n' { ' ' } else { c }) + .filter(|&c| { + !c.is_control() + // Bidi/format controls that could spoof or reorder the displayed name. + && !('\u{202A}'..='\u{202E}').contains(&c) // LRE..RLO/PDF + && !('\u{2066}'..='\u{2069}').contains(&c) // LRI..PDI + && c != '\u{200E}' // LRM + && c != '\u{200F}' // RLM + && c != '\u{061C}' // ALM + && c != '\u{FEFF}' // BOM / zero-width no-break space + }) + .collect(); + // Collapse internal whitespace runs, trim, cap at the wire limit. + let collapsed = cleaned.split_whitespace().collect::>().join(" "); + let mut trimmed = collapsed.as_str(); + while trimmed.len() > NAME_MAX { + let mut cut = NAME_MAX; + while !trimmed.is_char_boundary(cut) { + cut -= 1; + } + trimmed = &trimmed[..cut]; + } + let trimmed = trimmed.trim(); + if trimmed.is_empty() { + format!("device {}", &fp_hex[..8.min(fp_hex.len())]) + } else { + trimmed.to_string() + } +} + +/// Max stored device-name length (matches the `Hello` wire cap, `quic::HELLO_NAME_MAX`). +const NAME_MAX: usize = 64; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn sanitize_strips_control_and_bidi() { + // ANSI escape + newline + a bidi override that could spoof the displayed name. + let dirty = "\u{1b}]0;evil\u{07}Good\nDevice\u{202E}xfp"; + let clean = sanitize_device_name(dirty, "deadbeef00"); + assert!(!clean.contains('\u{1b}') && !clean.contains('\n') && !clean.contains('\u{202E}')); + // ESC dropped (']' survives), BEL dropped, '\n'→space (Good Device), RLO dropped (no space). + assert_eq!(clean, "]0;evilGood Devicexfp"); + // All-control / empty → fingerprint-derived fallback. + assert_eq!( + sanitize_device_name("\u{1b}\u{07}", "deadbeef00"), + "device deadbeef" + ); + assert_eq!(sanitize_device_name(" ", "abc"), "device abc"); + // Over-long names cap at a char boundary. + assert!(sanitize_device_name(&"x".repeat(200), "ab").len() <= 64); + } +}