diff --git a/apps/wordcraft/src/control_server.rs b/apps/wordcraft/src/control_server.rs index a30625a41..ce9f9dce6 100644 --- a/apps/wordcraft/src/control_server.rs +++ b/apps/wordcraft/src/control_server.rs @@ -31,32 +31,90 @@ pub fn start(port: u16, ctx: egui::Context) -> Receiver { fn serve(stream: TcpStream, tx: Sender, ctx: egui::Context) { let Ok(read) = stream.try_clone() else { return }; - let mut out = stream; - for line in BufReader::new(read).lines() { + serve_lines(BufReader::new(read), stream, &tx, &ctx); +} + +/// Answer requests until the peer hangs up or sends a line that isn't JSON. Closing on bad +/// input matters: an HTTP request (say, a web page's `fetch` to this port) starts with a +/// request line that never parses, so it can't carry a command in its body. +fn serve_lines(input: impl BufRead, mut out: impl Write, tx: &Sender, ctx: &egui::Context) { + for line in input.lines() { let Ok(line) = line else { break }; if line.trim().is_empty() { continue; } - let reply = match serde_json::from_str::(&line) { - Ok(msg) => { - let id = msg.get("id").cloned().unwrap_or(Value::Null); - let method = msg.get("method").and_then(Value::as_str).unwrap_or("").to_string(); - let params = msg.get("params").cloned().unwrap_or(json!({})); - let (req, rrx) = ControlRequest::new(method, params); - if tx.send(req).is_err() { - break; - } - ctx.request_repaint(); - let mut r = rrx.recv_timeout(Duration::from_secs(60)).unwrap_or_else(|_| json!({"ok": false, "error": "timeout"})); - if let Some(o) = r.as_object_mut() { - o.insert("id".into(), id); - } - r + let msg = match serde_json::from_str::(&line) { + Ok(msg) => msg, + Err(e) => { + let _ = writeln!(out, "{}", json!({"ok": false, "error": format!("bad JSON: {e}; closing the connection")})); + break; } - Err(e) => json!({"ok": false, "error": format!("bad JSON: {e}")}), }; + let id = msg.get("id").cloned().unwrap_or(Value::Null); + let method = msg.get("method").and_then(Value::as_str).unwrap_or("").to_string(); + let params = msg.get("params").cloned().unwrap_or(json!({})); + let (req, rrx) = ControlRequest::new(method, params); + if tx.send(req).is_err() { + break; + } + ctx.request_repaint(); + let mut reply = rrx.recv_timeout(Duration::from_secs(60)).unwrap_or_else(|_| json!({"ok": false, "error": "timeout"})); + if let Some(o) = reply.as_object_mut() { + o.insert("id".into(), id); + } if writeln!(out, "{reply}").is_err() { break; } } } + +#[cfg(test)] +mod tests { + use super::*; + + /// Run `input` through the server with an app that answers every request with its method + /// name; returns the methods the app received and the reply lines. + fn run(input: &str) -> (Vec, Vec) { + let (tx, rx) = channel::(); + let app = std::thread::spawn(move || { + let mut seen = Vec::new(); + for req in rx { + let _ = req.reply.send(json!({"ok": true, "result": req.method})); + seen.push(req.method); + } + seen + }); + let mut out = Vec::new(); + serve_lines(input.as_bytes(), &mut out, &tx, &egui::Context::default()); + drop(tx); + let seen = app.join().unwrap(); + let replies = String::from_utf8(out).unwrap().lines().map(|l| serde_json::from_str(l).unwrap()).collect(); + (seen, replies) + } + + #[test] + fn json_lines_get_replies() { + let (seen, replies) = run("{\"id\":1,\"method\":\"format.bold\"}\n\n{\"id\":2,\"method\":\"ui.inspect\"}\n"); + assert_eq!(seen, ["format.bold", "ui.inspect"]); + assert_eq!(replies.len(), 2); + assert_eq!(replies[1]["id"], 2); + assert_eq!(replies[1]["result"], "ui.inspect"); + } + + #[test] + fn bad_json_closes_the_connection() { + let (seen, replies) = run("not json\n{\"id\":1,\"method\":\"format.bold\"}\n"); + assert!(seen.is_empty()); + assert_eq!(replies.len(), 1); + assert_eq!(replies[0]["ok"], false); + } + + #[test] + fn http_request_cannot_smuggle_a_command() { + // What a web page's `fetch("http://127.0.0.1:/", {method: "POST", body})` sends. + let body = "\n{\"method\":\"file.saveAs\",\"params\":{\"path\":\"/tmp/x.docx\"}}\n"; + let req = format!("POST / HTTP/1.1\r\nHost: 127.0.0.1:7981\r\nContent-Type: text/plain\r\nContent-Length: {}\r\n\r\n{body}", body.len()); + let (seen, _) = run(&req); + assert!(seen.is_empty(), "{seen:?}"); + } +} diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index a3140416d..e57063ad0 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -514,8 +514,10 @@ fn mouse(app: &mut WordApp, ui: &Ui, resp: &egui::Response, rects: &[Rect], layo { if let Some(name) = link.strip_prefix('#') { let _ = app.run("edit.goto", json!({"bookmark": name})); - } else { + } else if is_followable_link(&link) { app.canvas.open_url = Some(link); + } else { + app.status(tl!("Only web (http, https) and email (mailto) links can be followed")); } return; } @@ -806,3 +808,38 @@ fn context_menu(app: &mut WordApp, ui: &mut Ui) { item(ui, app, "Merge Cells", "table.merge", json!({})); } } + +/// Whether Ctrl/⌘+click may hand a document's hyperlink to the system: web and email links only, +/// so a document can't launch `file:` paths, programs or custom-scheme handlers. +fn is_followable_link(url: &str) -> bool { + let Some((scheme, _)) = url.split_once(':') else { return false }; + ["http", "https", "mailto"].iter().any(|s| scheme.eq_ignore_ascii_case(s)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn only_web_and_email_links_are_followed() { + for ok in ["https://example.com/a?b#c", "http://example.com", "HTTPS://EXAMPLE.COM", "mailto:someone@example.com"] { + assert!(is_followable_link(ok), "{ok}"); + } + for bad in [ + "file:///etc/passwd", + "file://C:/Windows/System32/calc.exe", + "C:\\Windows\\System32\\calc.exe", + "/usr/bin/xterm", + "javascript:alert(1)", + "ms-msdt:/id", + "smb://host/share", + " https://example.com", + "\thttps://example.com", + "https\u{0}://example.com", + "", + "example.com", + ] { + assert!(!is_followable_link(bad), "{bad:?}"); + } + } +} diff --git a/crates/ui-egui/src/i18n/ja.tsv b/crates/ui-egui/src/i18n/ja.tsv index a418dba85..9e153c5f5 100644 --- a/crates/ui-egui/src/i18n/ja.tsv +++ b/crates/ui-egui/src/i18n/ja.tsv @@ -437,6 +437,7 @@ OK OK One 1 段 One Page 1 ページ + Only web (http, https) and email (mailto) links can be followed 開けるのは Web(http、https)とメール(mailto)のリンクだけです Open 開く OpenDocument Text (*.odt) OpenDocument テキスト (*.odt) Optional Hyphen 任意指定のハイフン diff --git a/crates/ui-egui/src/i18n/zh-hans.tsv b/crates/ui-egui/src/i18n/zh-hans.tsv index ebb9bba4d..a37e52bd9 100644 --- a/crates/ui-egui/src/i18n/zh-hans.tsv +++ b/crates/ui-egui/src/i18n/zh-hans.tsv @@ -448,6 +448,7 @@ OK 确定 One 一栏 One Page 单页 + Only web (http, https) and email (mailto) links can be followed 只能打开网页(http、https)和电子邮件(mailto)链接 Open 打开 OpenDocument Text (*.odt) OpenDocument 文本 (*.odt) Optional Hyphen 可选连字符 diff --git a/crates/ui-egui/src/i18n/zh-hant.tsv b/crates/ui-egui/src/i18n/zh-hant.tsv index 982e3b86c..0893e09c6 100644 --- a/crates/ui-egui/src/i18n/zh-hant.tsv +++ b/crates/ui-egui/src/i18n/zh-hant.tsv @@ -438,6 +438,7 @@ OK 確定 One 一欄 One Page 單頁 + Only web (http, https) and email (mailto) links can be followed 只能開啟網頁(http、https)和電子郵件(mailto)連結 Open 開啟 OpenDocument Text (*.odt) OpenDocument 文字 (*.odt) Optional Hyphen 選擇性連字號 diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 11d55b01d..207bec340 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -9,7 +9,8 @@ It listens on `127.0.0.1` only. Send one JSON object per line; each gets one rep ``` Errors come back as `{"id": …, "ok": false, "error": "message"}`. A failed command leaves the -document unchanged. +document unchanged. A line that isn't valid JSON gets one error reply, then the server closes the +connection (so an HTTP request sent to the port can't run a command); reconnect to continue. ## Methods