diff --git a/ATTRIBUTION.toml b/ATTRIBUTION.toml index 0d355408..5549859e 100644 --- a/ATTRIBUTION.toml +++ b/ATTRIBUTION.toml @@ -19,7 +19,7 @@ licence = "MIT OR Apache-2.0" licence_file = "LICENSE-MIT" kind = "translation" usage = "Simplified Chinese UI strings, compiled in by crates/ui-egui/src/i18n/mod.rs" -sha256 = "cac5e47a10ff49a6d41d87869fda1760778b533319f8ccec2b4e8e67b8a8e0dd" +sha256 = "d55df478f60a8a8185cdbc428d7f1c276b7ab21a8a4024faf0e978387fd3189e" [[asset]] path = "crates/ui-egui/src/i18n/zh-hant.tsv" @@ -63,7 +63,7 @@ licence = "MIT OR Apache-2.0" licence_file = "LICENSE-MIT" kind = "translation" usage = "Spanish UI strings, compiled in by crates/ui-egui/src/i18n/mod.rs" -sha256 = "6d2da5a55e6eb10e26efbdd4d34e48be30c2cad8e3f43e1fe811d8ce95dd0a19" +sha256 = "51982e7f71c598aa1fd23f1883cc84ee69ca15b4cd31671015328fb18e187e0b" [[asset]] path = "crates/ui-egui/src/i18n/fr.tsv" @@ -74,7 +74,7 @@ licence = "MIT OR Apache-2.0" licence_file = "LICENSE-MIT" kind = "translation" usage = "French UI strings, compiled in by crates/ui-egui/src/i18n/mod.rs" -sha256 = "2be55ac9d1303fea2d61f3f6c594cf5003e2b2aa405eefddffed2ea158a94b5e" +sha256 = "41b46b0540aead4c11b4a30b72758f6fa90888a544c18108a8c55d3682e1da2a" [[asset]] path = "crates/ui-egui/src/i18n/ja.tsv" diff --git a/Cargo.lock b/Cargo.lock index 2520b60a..e229f2c6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3573,6 +3573,8 @@ name = "pdfcraft-engine" version = "0.4.0" dependencies = [ "chrono", + "icu_properties", + "idna", "image", "pdfcraft-a11y", "pdfcraft-annot", @@ -3598,6 +3600,7 @@ dependencies = [ "pdfcraft-xfdf", "png 0.18.1", "thiserror 2.0.21", + "url", ] [[package]] diff --git a/crates/engine/Cargo.toml b/crates/engine/Cargo.toml index 272698ea..5fbc9a4b 100644 --- a/crates/engine/Cargo.toml +++ b/crates/engine/Cargo.toml @@ -34,6 +34,11 @@ pdfcraft-create = { workspace = true } pdfcraft-model = { workspace = true } thiserror = { workspace = true } png = "0.18" +# Already in the graph (url and idna through webbrowser, icu_properties through boa): they show +# the host of a document's link the way the browser reads it (links::display_host). +url = "2.5" +idna = "1.1" +icu_properties = "2.3" [target.'cfg(not(target_arch = "wasm32"))'.dependencies] # Already in the graph through lopdf; used only for the local UTC offset ("today"). diff --git a/crates/engine/src/links.rs b/crates/engine/src/links.rs index 8255daa4..683d5648 100644 --- a/crates/engine/src/links.rs +++ b/crates/engine/src/links.rs @@ -1,6 +1,8 @@ //! Where PdfCraft and the ArtCraft community live on the web. One table, so the Help menu, the //! About dialog, the home screen, the CLI and the README agree. +use icu_properties::props::Script; + /// The app's name in ArtCraft URLs (`getartcraft.com/apps/{APP}`, `github.com/storytold/{APP}`). pub const APP: &str = "pdfcraft"; @@ -53,6 +55,13 @@ pub enum BlockedLink { Malformed, /// A scheme other than [`DOCUMENT_SCHEMES`] (lowercased). Scheme(String), + /// An email address asking the email app to attach a local file (`attach=`, `attachment=`) + /// or read one into the message (`insert=`). + MailFile, + /// An email address containing text that some email tools decode before reading the address, + /// so it could spell `attach`: an RFC 2047 encoded word (`=?utf-8?q?…?=`) or a backslash + /// escape (`\0141`, `\x61`). + MailEncodedWord, } impl std::fmt::Display for BlockedLink { @@ -63,6 +72,8 @@ impl std::fmt::Display for BlockedLink { Self::Hidden => write!(f, "the address contains hidden or invisible characters"), Self::Malformed => write!(f, "the address isn't a complete web or email address"), Self::Scheme(s) => write!(f, "it is a “{s}:” address"), + Self::MailFile => write!(f, "it asks your email app to attach or insert a file from your computer"), + Self::MailEncodedWord => write!(f, "it contains encoded text that some email apps turn into other instructions"), } } } @@ -103,14 +114,106 @@ pub fn document_url(raw: &str) -> Result { } let complete = match scheme.as_str() { "mailto" => !rest.is_empty(), - _ => authority(url).is_some_and(|a| !a.contains(' ')) && host(url).is_some(), + _ => authority(url).is_some_and(|a| !a.contains(' ')) && display_host(url).is_some(), }; - if !complete { + // The desktop app opens addresses through the webbrowser crate, which treats one the WHATWG URL + // parser rejects as a local file path and opens that instead (`mailto://a^b/` would become + // `file:///…/mailto:/a^b/`). So only an address the parser accepts, as the same scheme, goes on. + if !complete || !url::Url::parse(url).is_ok_and(|u| u.scheme() == scheme) { return Err(BlockedLink::Malformed); } + if scheme == "mailto" { + check_mail_parameters(rest)?; + } Ok(url.to_string()) } +/// Email-address parameters that make an email app read a file from the user's computer: Evolution, +/// KMail and Claws Mail attach the file named by `attach` (Evolution and KMail also by +/// `attachment`), and Claws Mail reads an `insert` file into the message body. Names are matched as +/// prefixes, so `attachments` is refused too. +const MAIL_FILE_PARAMETERS: &[&str] = &["attach", "insert"]; + +/// How many layers of percent-encoding are looked through. Email apps decode once at most. +const MAIL_DECODE_LAYERS: usize = 8; + +/// Refuse an email address (what follows `mailto:`) that asks for a local file. A document can't +/// need this, and a drafted message carrying the user's files is one click from being sent. +/// +/// Email apps disagree on how they read the address, so this doesn't try to parse it the way any +/// one of them does. Every email app reads a parameter as `name=value`, so it refuses any word +/// that starts with one of [`MAIL_FILE_PARAMETERS`] (in any case) and is followed by `=`, wherever +/// it stands: after `?`, `&`, `#` or `;` (KMail looks for the first `?` anywhere and reads `&` +/// as `&`), with no `?` at all (xdg-email reads `mailto:a@example.org&attach=…` as a parameter) or +/// after a comma (xdg-email copies the subject into Thunderbird's comma-separated `-compose` +/// argument). The check is repeated after each layer of percent-decoding (KMail decodes names, so +/// `%61ttach` is `attach`). A word without `=` (`Please attach the invoice`) is fine. +/// +/// The address is refused, not stripped of the parameter: PdfCraft never rewrites what a document +/// asks to open, and a stripped address could still be read differently by some email app. +fn check_mail_parameters(rest: &str) -> Result<(), BlockedLink> { + let mut text = rest.to_string(); + for _ in 0..MAIL_DECODE_LAYERS { + check_mail_layer(&text)?; + let decoded = String::from_utf8_lossy(&percent_decode(text.as_bytes())).into_owned(); + if decoded == text { + return Ok(()); + } + text = decoded; + } + check_mail_layer(&text) +} + +/// One layer of [`check_mail_parameters`]. +fn check_mail_layer(text: &str) -> Result<(), BlockedLink> { + // KMail decodes RFC 2047 encoded words (`=?utf-8?q?attach?=`) across the whole address before + // reading it, and its decoder is lenient, so any `=?` followed later by `?=` is refused. + if text.find("=?").is_some_and(|i| text.get(i + 2..).is_some_and(|after| after.contains("?="))) { + return Err(BlockedLink::MailEncodedWord); + } + // xdg-email passes the address through `echo`, which turns `\0141` (octal) or `\x61` into `a`. + if text.split('\\').skip(1).any(|after| after.starts_with(|c: char| c.is_ascii_digit() || matches!(c, 'x' | 'X'))) { + return Err(BlockedLink::MailEncodedWord); + } + let lower = text.to_ascii_lowercase(); + let is_word = |c: char| c.is_ascii_alphanumeric() || matches!(c, '_' | '-'); + let mut previous = None; + for (i, c) in lower.char_indices() { + let starts_word = is_word(c) && !previous.is_some_and(is_word); + previous = Some(c); + if !starts_word { + continue; + } + let Some(word) = lower.get(i..) else { continue }; + let name_end = word.find(|c: char| !is_word(c)).unwrap_or(word.len()); + let (name, after) = word.split_at(name_end); + let asks = MAIL_FILE_PARAMETERS.iter().any(|p| name.starts_with(p)); + if asks && after.trim_start_matches([' ', '\t', '+', '\'', '"']).starts_with('=') { + return Err(BlockedLink::MailFile); + } + } + Ok(()) +} + +/// Decode `%XX` escapes. A `%` not followed by two hex digits is kept as it is. +fn percent_decode(s: &[u8]) -> Vec { + let hex = |b: Option<&u8>| b.and_then(|b| char::from(*b).to_digit(16)).and_then(|d| u8::try_from(d).ok()); + let mut out = Vec::with_capacity(s.len()); + let mut i = 0; + while let Some(&b) = s.get(i) { + if b == b'%' + && let (Some(high), Some(low)) = (hex(s.get(i + 1)), hex(s.get(i + 2))) + { + out.push((high << 4) | low); + i += 3; + } else { + out.push(b); + i += 1; + } + } + out +} + /// The authority of a web address: what follows `//`, up to the first `/`, `\`, `?` or `#`. /// Browsers read `\` as `/` in web addresses (the WHATWG URL standard), so it ends the authority /// too; otherwise `https://other.example\@trusted.example/` would seem to go to @@ -138,6 +241,102 @@ pub fn host(url: &str) -> Option<&str> { (!host.is_empty()).then_some(host) } +/// How to show a web address's host so that a lookalike can't pass for a familiar site. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct HostDisplay { + /// The host the browser connects to: lowercase ASCII, with international labels in punycode + /// (`xn--…`) and numeric addresses in their usual form (`192.168.1.1`, `[::1]`). Show this one, + /// not the document's spelling. + pub ascii: String, + /// The host has a label in another alphabet (`xn--…`), whose letters can look like familiar + /// ones. + pub international: bool, + /// A label mixes writing systems, such as the Cyrillic `а` in `pаypal.com`: a common way to + /// imitate another site's name. + pub mixed_scripts: bool, +} + +/// The host of a web address as the browser will see it (see [`HostDisplay`]). On the desktop the +/// address is handed to the operating system as the `url` crate (the WHATWG URL standard) +/// serialises it, and in the browser build the browser parses it by the same standard, so this +/// reads the host with that crate: percent-decoded, IDNA-mapped (case, fullwidth letters, `。`) and +/// with numeric hosts such as `0x7f.1` resolved. `None` for a non-web address, one without a host +/// (see [`host`]), or one the browser would reject. +pub fn display_host(url: &str) -> Option { + host(url)?; + let parsed = url::Url::parse(url).ok()?; + let ascii = match parsed.host()? { + url::Host::Domain(d) => d.to_string(), + url::Host::Ipv4(a) => a.to_string(), + url::Host::Ipv6(a) => format!("[{a}]"), + }; + if !ascii.split('.').any(|label| label.starts_with("xn--")) { + return Some(HostDisplay { ascii, international: false, mixed_scripts: false }); + } + let (unicode, valid) = idna::domain_to_unicode(&ascii); + let mixed_scripts = valid.is_err() || unicode.split('.').any(mixes_scripts); + Some(HostDisplay { ascii, international: true, mixed_scripts }) +} + +/// A writing system for comparing the characters of a label. Han characters are written with +/// Japanese, Korean and Chinese ones, so those three combinations count as one system each +/// (Unicode TR 39's augmented script sets). +#[derive(Clone, Copy, PartialEq, Eq)] +enum Writing { + Script(Script), + Japanese, + Korean, + Chinese, +} + +/// The writing systems a character belongs to, or `None` for one shared by all (digits, `-`, +/// combining marks). +fn writing_systems(c: char) -> Option> { + let scripts = icu_properties::script::ScriptWithExtensions::new().get_script_extensions_val(c); + let mut out = Vec::new(); + for s in scripts.iter() { + if s == Script::Common || s == Script::Inherited { + return None; + } + out.push(Writing::Script(s)); + match s { + Script::Han => out.extend([Writing::Japanese, Writing::Korean, Writing::Chinese]), + Script::Hiragana | Script::Katakana => out.push(Writing::Japanese), + Script::Hangul => out.push(Writing::Korean), + Script::Bopomofo => out.push(Writing::Chinese), + _ => {} + } + } + Some(out) +} + +/// The writing systems every character of `label` belongs to (Unicode TR 39's resolved script +/// set), skipping characters that `skip` picks. `None` when no character narrows it down. +fn shared_writing(label: &str, skip: impl Fn(&[Writing]) -> bool) -> Option> { + let mut shared: Option> = None; + for systems in label.chars().filter_map(writing_systems) { + if skip(&systems) { + continue; + } + shared = Some(match shared { + None => systems, + Some(s) => s.into_iter().filter(|w| systems.contains(w)).collect(), + }); + } + shared +} + +/// Whether a label mixes writing systems. Latin alongside Japanese, Korean or Chinese is allowed, as +/// in Unicode TR 39's "highly restrictive" level (`ソニーstore`). +fn mixes_scripts(label: &str) -> bool { + if shared_writing(label, |_| false).is_none_or(|s| !s.is_empty()) { + return false; + } + let latin = Writing::Script(Script::Latin); + let rest = shared_writing(label, |s| s.contains(&latin)).unwrap_or_default(); + !rest.iter().any(|w| matches!(w, Writing::Japanese | Writing::Korean | Writing::Chinese)) +} + #[cfg(test)] mod tests { #[test] @@ -223,4 +422,136 @@ mod tests { assert_eq!(host("mailto:a@example.org"), None); assert_eq!(host("https:///nohost"), None); } + + #[test] + fn addresses_the_url_parser_rejects_are_refused() { + use super::{BlockedLink as B, document_url}; + // The webbrowser crate would open each of these as a local file path. + for bad in ["mailto://a^b/x", "mailto://a:99999/", "mailto://[x]/", "mailto://a\\b/", "mailto://a b/", "https://a^b/", "http://a self.notify_fmt( - "{who} in this document tried to open an address PdfCraft won't open: {e}. Only web (http, https) and email (mailto) links open from documents.", - &[("who", tl!(origin.noun())), ("e", &e.to_string())], - ), + Err(e) => { + use pdfcraft_engine::links::BlockedLink; + let template = if matches!(e, BlockedLink::MailFile | BlockedLink::MailEncodedWord) { + "{who} in this document tried to open an address PdfCraft won't open: {e}." + } else { + "{who} in this document tried to open an address PdfCraft won't open: {e}. Only web (http, https) and email (mailto) links open from documents." + }; + self.notify_fmt(template, &[("who", tl!(origin.noun())), ("e", &e.to_string())]); + } } } diff --git a/crates/ui-egui/tests/document_links.rs b/crates/ui-egui/tests/document_links.rs index b7857ba8..a866a916 100644 --- a/crates/ui-egui/tests/document_links.rs +++ b/crates/ui-egui/tests/document_links.rs @@ -156,7 +156,17 @@ fn a_script_asks_first_and_only_once() { #[test] fn a_script_cannot_open_other_kinds_of_address() { let _gpu = gpu(); - for url in ["file:///etc/passwd", "javascript:alert(1)", "ms-settings:privacy", "smb://server/share", "https://exa\u{202E}gro.elpmaxe"] { + // `mailto://a^b/x` and `https://a^b/` don't parse as URLs, so the browser would be handed them + // as local file paths. + for url in [ + "file:///etc/passwd", + "javascript:alert(1)", + "ms-settings:privacy", + "smb://server/share", + "https://exa\u{202E}gro.elpmaxe", + "mailto://a^b/x", + "https://a^b/", + ] { let mut h = harness(); let id = h.state().views[0].id; let script = format!("app.launchURL({});", serde_json::to_string(url).unwrap()); @@ -167,3 +177,58 @@ fn a_script_cannot_open_other_kinds_of_address() { assert!(h.state().toast.clone().is_some_and(|t| t.0.contains("won't open")), "{url}: {:?}", h.state().toast); } } + +#[test] +fn an_email_link_cannot_attach_a_local_file() { + let _gpu = gpu(); + for url in ["mailto:a@example.org?attach=/home/me/.ssh/id_ed25519", "mailto:a@example.org?subject=Hi&%61ttachment=C:/Users/me/x.txt"] { + let mut h = harness(); + let id = h.state().views[0].id; + let script = format!("app.launchURL({});", serde_json::to_string(url).unwrap()); + h.state_mut().run_button_script(id, "web", &script); + h.run_steps(3); + assert_eq!(h.state().pending_link, None, "{url}"); + assert_eq!(h.state().last_opened_url, None, "{url}"); + let toast = h.state().toast.clone().expect("the user is told").0; + assert!(toast.contains("attach or insert a file"), "{url}: {toast}"); + } +} + +/// Ask to open `url` from a script and return the harness with the prompt showing. +fn prompt_for(url: &str) -> Harness<'static, PdfCraftApp> { + let mut h = harness(); + let id = h.state().views[0].id; + let script = format!("app.launchURL({});", serde_json::to_string(url).unwrap()); + h.state_mut().run_button_script(id, "web", &script); + h.run_steps(3); + assert_eq!(h.state().pending_link, pending(url, LinkOrigin::Script)); + h +} + +#[test] +fn a_lookalike_host_is_shown_in_punycode_and_flagged() { + let _gpu = gpu(); + // `pаypal.com` with a Cyrillic `а` (U+0430). + let h = prompt_for("https://p\u{0430}ypal.com/login"); + h.get_by_label("xn--pypal-4ve.com"); + assert_eq!(h.query_all_by_label_contains("p\u{0430}ypal.com").count(), 1, "only the full address shows the lookalike form"); + h.get_by_label_contains("mixes letters from different alphabets"); +} + +#[test] +fn an_international_host_is_shown_in_punycode_and_marked() { + let _gpu = gpu(); + let h = prompt_for("https://bücher.example/"); + h.get_by_label("xn--bcher-kva.example"); + assert_eq!(h.query_all_by_label_contains("bücher.example").count(), 1, "only the full address shows the Unicode form"); + h.get_by_label_contains("letters from another alphabet"); + assert!(h.query_by_label_contains("mixes letters").is_none()); +} + +#[test] +fn a_numeric_host_is_shown_as_the_address_it_goes_to() { + let _gpu = gpu(); + let h = prompt_for("http://3232235777/admin"); + h.get_by_label("192.168.1.1"); + assert!(h.query_by_label_contains("alphabet").is_none()); +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index f5ddc31c..1f9e8154 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -2202,8 +2202,8 @@ milestone = "M7" status = "shipped" commands = ["edit.link"] tools = ["link_add"] -evidence = ["crates/annot/src/tests.rs::links_are_added_edited_listed_and_removed", "crates/automation/tests/automation.rs::links_through_tools", "crates/ui-egui/tests/document_links.rs::a_link_asks_first_and_opens_when_allowed", "crates/ui-egui/tests/document_links.rs::a_button_uri_action_asks_first"] -notes = "Opening a document's link or button URI asks first; only http, https and mailto addresses are offered (#90, #91)." +evidence = ["crates/annot/src/tests.rs::links_are_added_edited_listed_and_removed", "crates/automation/tests/automation.rs::links_through_tools", "crates/ui-egui/tests/document_links.rs::a_link_asks_first_and_opens_when_allowed", "crates/ui-egui/tests/document_links.rs::a_button_uri_action_asks_first", "crates/ui-egui/tests/document_links.rs::an_email_link_cannot_attach_a_local_file", "crates/ui-egui/tests/document_links.rs::a_lookalike_host_is_shown_in_punycode_and_flagged"] +notes = "Opening a document's link or button URI asks first; only http, https and mailto addresses are offered (#90, #91), and an email address that asks the email app to attach or insert a local file is refused. The prompt shows the host as the browser receives it: international hosts in punycode, lookalikes that mix alphabets flagged, numeric hosts in their usual form." [[feature]] id = "edit.action-launch"