#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) <noreply@anthropic.com>
This commit is contained in:
Martin Ball
2026-10-25 23:49:08 +00:00
co-authored by Claude Opus 5.5
parent ab26f22c4a
commit d8d75e4473
+33 -3
View File
@@ -657,14 +657,14 @@ fn run<S: AsRef<OsStr>>(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());