Merge pull request #1936 from storytold/fix/sec-doc-url-shell-chars

fix(engine): refuse document URLs with shell syntax or a malformed authority

crates/engine/src/automation/doc_urls.rs

@@ -28,6 +28,11 @@ pub fn openable(url: &str) -> bool {
     if url.is_empty() || url.chars().any(char::is_control) {
         return false;
     }
+    // Never valid in a URL unencoded, and read as syntax by the shells some openers go through
+    // (webbrowser on WSL hands the address to `cmd.exe /c start`, escaping only `&` and `^`).
+    if url.chars().any(|c| c.is_whitespace() || matches!(c, '|' | '<' | '>' | '"' | '`' | '\\' | '^')) {
+        return false;
+    }
     let Some((scheme, rest)) = url.split_once(':') else { return false };
     // An RFC 3986 scheme, so a stray colon in text cannot look like one.
     let mut chars = scheme.chars();
@@ -44,5 +49,11 @@ pub fn openable(url: &str) -> bool {
     // `http` and `https` name a host, so the authority is required: `http:x` is not a web link,
     // and `http:///p` has no host.
     let Some(after) = rest.strip_prefix("//") else { return false };
-    !after.is_empty() && !after.starts_with('/')
+    if after.is_empty() || after.starts_with('/') {
+        return false;
+    }
+    // The authority is made of host characters only (an opener that can't parse the address may
+    // fall back to treating it as a file path).
+    let authority = after.split(['/', '?', '#']).next().unwrap_or_default();
+    authority.chars().all(|c| !c.is_ascii() || c.is_ascii_alphanumeric() || "-._~:@[]%!$&'()*+,;=".contains(c))
 }

crates/engine/src/doc_urls_tests.rs

@@ -85,3 +85,25 @@ fn a_scheme_is_required_and_control_characters_are_refused() {
         assert!(!openable(url), "{url:?} must be refused");
     }
 }
+
+#[test]
+fn shell_syntax_and_malformed_authorities_are_refused() {
+    for url in [
+        // Never valid unencoded, and command syntax to the `cmd.exe /c start` an opener may use.
+        "https://example.com/?a=b|calc",
+        "https://example.com/<x>",
+        "https://example.com/\"x\"",
+        "https://example.com/a b",
+        "https://example.com/`x`",
+        "https://example.com/x^y",
+        "https://example.com\\x",
+        // An authority an opener can't parse may be reopened as a file path.
+        "http://exam ple.com/",
+        "http://a{b}/",
+    ] {
+        assert!(!openable(url), "{url:?} must be refused");
+    }
+    for url in ["https://example.com/?a=b&c=d%20e", "https://[::1]:8080/x", "https://bücher.example/", "https://a.b-c_d.example:443/"] {
+        assert!(openable(url), "{url:?} should open");
+    }
+}