From d8d75e44739370102ff53be42d5fdc857b08bc0d Mon Sep 17 00:00:00 2001 From: Martin Ball Date: Thu, 22 Oct 2026 19:21:10 +0100 Subject: [PATCH] #3692: wordcraft-cli zotero escapes peer text on stderr Audit finding 79 (CWE-117): the CLI printed Zotero's alert text (and, with --trace, the method name and call/reply JSON) to stderr as received, so a peer on Zotero's port could end the "Zotero:" line, forge following output or send terminal escape sequences. escape_peer shows backslashes, control characters (C0, DEL, C1), line separators and bidi overrides as visible escapes (\\, \n, \u{1b}), keeping each record on its own attributable line. Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/wordcraft-cli/src/main.rs | 36 +++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/apps/wordcraft-cli/src/main.rs b/apps/wordcraft-cli/src/main.rs index 752f9f68b..0817d18b3 100644 --- a/apps/wordcraft-cli/src/main.rs +++ b/apps/wordcraft-cli/src/main.rs @@ -657,14 +657,14 @@ fn run>(args: &[S]) -> Result<(), Failure> { let out = wordcraft_zotero::client::run_command(&opts, command, &mut |call| { let r = bridge.handle(&mut s, &mut host, call); if trace { - eprintln!("← {} {}", call.method, clip(&Value::Array(call.args.clone()).to_string())); - eprintln!("→ {}", clip(&String::from_utf8_lossy(&wordcraft_zotero::wire::encode_reply(&r)))); + eprintln!("← {} {}", escape_peer(&call.method), escape_peer(&clip(&Value::Array(call.args.clone()).to_string()))); + eprintln!("→ {}", escape_peer(&clip(&String::from_utf8_lossy(&wordcraft_zotero::wire::encode_reply(&r))))); } r }) .map_err(|e| e.to_string())?; for a in &host.alerts { - eprintln!("Zotero: {a}"); + eprintln!("Zotero: {}", escape_peer(a)); } eprintln!("{} calls, {}", out.calls, if out.completed { "completed" } else { "ended without completing (cancelled?)" }); if let Some(o) = arg_value(&rest, "--save") { @@ -687,6 +687,27 @@ fn clip(s: &str) -> String { } } +/// Text from another program (Zotero, or whatever answered on its port) for one diagnostic +/// line: backslashes, control characters (newline, escape, C1…), line separators and bidi +/// overrides are shown as escapes (`\\`, `\n`, `\u{1b}`), so it can't end its line, drive the +/// terminal or pass for other output. +fn escape_peer(s: &str) -> String { + let mut out = String::with_capacity(s.len()); + for c in s.chars() { + match c { + '\\' => out.push_str("\\\\"), + '\n' => out.push_str("\\n"), + '\r' => out.push_str("\\r"), + '\t' => out.push_str("\\t"), + c if c.is_control() || matches!(c, '\u{061c}' | '\u{200e}' | '\u{200f}' | '\u{2028}'..='\u{202e}' | '\u{2066}'..='\u{2069}') => { + out.extend(c.escape_unicode()) + } + c => out.push(c), + } + } + out +} + fn parity_markdown(p: &Value) -> String { let mut s = String::from( "# WordCraft feature parity\n\nGenerated by `cargo xtask parity` (`wordcraft-cli parity --markdown`): the word-processor feature catalog (`crates/engine/src/catalog.rs`) compared with the live command registry.\n\n", @@ -733,6 +754,15 @@ mod tests { values.iter().map(|value| (*value).into()).collect() } + #[test] + fn peer_text_stays_on_one_visible_line() { + let alert = "Done.\nwrote /etc/passwd\r\u{1b}[2J\u{9b}31m\u{202e}txt.exe\\n\tok é"; + let shown = escape_peer(alert); + assert_eq!(shown, "Done.\\nwrote /etc/passwd\\r\\u{1b}[2J\\u{9b}31m\\u{202e}txt.exe\\\\n\\tok é"); + assert!(!shown.chars().any(|c| c.is_control())); + assert_eq!(escape_peer("Citation (Doe, 2020) — “ok”"), "Citation (Doe, 2020) — “ok”"); + } + #[test] fn validators_accept_only_the_documented_options() { assert!(validate_options("info", &args(&["file.docx"]), PASSWORD_OPTIONS).is_ok());