fix: prevent launcher option and shell injection (#124)

Opening an attacker-controlled dash-leading path could be interpreted as
launcher options. On Windows, cmd /c start also parsed embedded quotes and
metacharacters as command language, while the legacy gnome-open fallback
could load a module even after a double-dash separator.

Add command-construction regressions for malicious option-shaped paths and
Windows shell metacharacters. Use supported separators on macOS and KDE, and
rewrite dash-leading relative paths for launchers without separator support.
Keep Windows values out of shell syntax by passing the default target through
the environment and invoking custom applications directly, with explorer.exe
as a PowerShell-free fallback.

Exclude cmd-based opening by default, while providing an
explicit insecure Cargo feature for users who need compatibility it
and accept their unsafe handling of untrusted input.

Validated with default and all-feature cargo tests, clippy, and cross-target
cargo check --tests for aarch64 Linux and Windows.

Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>

Author: codex

Cargo.toml

@@ -13,6 +13,12 @@ include = ["src/**/*", "LICENSE.md", "README.md", "changelog.md"]
 rust-version = "1.62"
 
 [features]
+## Restore the historical `cmd /c start` launcher on Windows. It cannot safely
+## receive untrusted paths because `cmd` interprets its arguments as shell syntax.
+##
+## Do not enable this feature when paths or URLs may be attacker-controlled.
+insecure = []
+
 ## If enabled, link to `system` on Windows and use `ShellExecuteW` intead of a command invocation
 ## when launching something in 'detached' mode.
 ## That way, it should be possible to open currently opened (for writing) files as well.

src/lib.rs

@@ -85,6 +85,17 @@
 //!
 //! It cannot be overemphasized how fragile this all is in UNIX environments. It is common for the various MIME tables to incorrectly specify the application "owning" a given filetype.
 //! It is common for openers to behave strangely. Use with caution, as this crate merely inherits a particular platforms shortcomings.
+//!
+//! ## Security boundaries
+//!
+//! Paths passed to system launchers are separated from launcher options where the
+//! launcher supports it. Applications selected through [`with()`] have arbitrary
+//! command-line grammars, however, so the crate cannot guarantee that every custom
+//! application treats a dash-leading path as data. Launching a document or custom
+//! URL scheme also trusts the configured local handler and the launched content.
+//! The `insecure` Cargo feature restores the legacy `cmd /c start` launcher on
+//! Windows for compatibility, but must not be enabled when paths or URLs may be
+//! attacker-controlled.
 
 #[cfg(target_os = "windows")]
 use windows as os;

src/macos.rs

@@ -2,12 +2,36 @@ use std::{ffi::OsStr, process::Command};
 
 pub fn commands<T: AsRef<OsStr>>(path: T) -> Vec<Command> {
     let mut cmd = Command::new("/usr/bin/open");
-    cmd.arg(path.as_ref());
+    cmd.arg("--").arg(path.as_ref());
     vec![cmd]
 }
 
 pub fn with_command<T: AsRef<OsStr>>(path: T, app: impl Into<String>) -> Command {
     let mut cmd = Command::new("/usr/bin/open");
-    cmd.arg(path.as_ref()).arg("-a").arg(app.into());
+    cmd.arg("-a").arg(app.into()).arg("--").arg(path.as_ref());
     cmd
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn separates_paths_from_open_options() {
+        assert_eq!(
+            commands("-aCalculator")[0].get_args().collect::<Vec<_>>(),
+            [OsStr::new("--"), OsStr::new("-aCalculator")]
+        );
+        assert_eq!(
+            with_command("-aCalculator", "Preview")
+                .get_args()
+                .collect::<Vec<_>>(),
+            [
+                OsStr::new("-a"),
+                OsStr::new("Preview"),
+                OsStr::new("--"),
+                OsStr::new("-aCalculator")
+            ]
+        );
+    }
+}

src/unix.rs

@@ -1,4 +1,24 @@
-use std::{ffi::OsStr, process::Command};
+use std::{
+    ffi::{OsStr, OsString},
+    process::Command,
+};
+
+/// Prevent launchers from interpreting a path that starts with `-` as an option.
+///
+/// Prefixing such paths with `./` preserves their meaning as paths relative to the
+/// current directory. This is used for launchers that do not support `--` as an
+/// end-of-options marker.
+fn option_safe_path(path: &OsStr) -> OsString {
+    use std::os::unix::ffi::OsStrExt;
+
+    if path.as_bytes().first() == Some(&b'-') {
+        let mut safe = OsString::from("./");
+        safe.push(path);
+        safe
+    } else {
+        path.to_os_string()
+    }
+}
 
 pub fn commands<T: AsRef<OsStr>>(path: T) -> Vec<Command> {
     let path = path.as_ref();
@@ -9,18 +29,23 @@ pub fn commands<T: AsRef<OsStr>>(path: T) -> Vec<Command> {
         commands.push(crate::wsl::command(path));
     }
 
-    let fallback_commands = [
-        ("xdg-open", vec![path]),
-        ("gio", vec![OsStr::new("open"), path]),
-        ("gnome-open", vec![path]),
-        ("kde-open", vec![path]),
-    ];
+    let safe_path = option_safe_path(path);
 
-    commands.extend(fallback_commands.iter().map(|(command, args)| {
-        let mut cmd = Command::new(command);
-        cmd.args(args);
-        cmd
-    }));
+    let mut xdg = Command::new("xdg-open");
+    xdg.arg(&safe_path);
+    commands.push(xdg);
+
+    let mut gio = Command::new("gio");
+    gio.arg("open").arg(&safe_path);
+    commands.push(gio);
+
+    let mut gnome = Command::new("gnome-open");
+    gnome.arg(&safe_path);
+    commands.push(gnome);
+
+    let mut kde = Command::new("kde-open");
+    kde.arg("--").arg(path);
+    commands.push(kde);
 
     commands
 }
@@ -30,3 +55,62 @@ pub fn with_command<T: AsRef<OsStr>>(path: T, app: impl Into<String>) -> Command
     cmd.arg(path.as_ref());
     cmd
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn args(command: &Command) -> Vec<&OsStr> {
+        command.get_args().collect()
+    }
+
+    #[test]
+    fn protects_dash_leading_paths_from_launcher_option_parsing() {
+        let commands = commands("--load-modules=/tmp/evil.so");
+
+        let expected_len = 4;
+        let commands = &commands[commands.len() - expected_len..];
+        assert_eq!(commands.len(), expected_len);
+        assert_eq!(commands[0].get_program(), "xdg-open");
+        assert_eq!(
+            args(&commands[0]),
+            [OsStr::new("./--load-modules=/tmp/evil.so")],
+            "xdg-open has no end-of-options marker, so the path must not start with a dash"
+        );
+        assert_eq!(commands[1].get_program(), "gio");
+        assert_eq!(
+            args(&commands[1]),
+            [
+                OsStr::new("open"),
+                OsStr::new("./--load-modules=/tmp/evil.so")
+            ],
+            "gio has no end-of-options marker, so the path must not start with a dash"
+        );
+        let kde = commands.last().unwrap();
+        assert_eq!(commands[2].get_program(), "gnome-open");
+        assert_eq!(
+            args(&commands[2]),
+            [OsStr::new("./--load-modules=/tmp/evil.so")],
+            "gnome-open would otherwise interpret the path as a module-loading option"
+        );
+        assert_eq!(kde.get_program(), "kde-open");
+        assert_eq!(
+            args(kde),
+            [OsStr::new("--"), OsStr::new("--load-modules=/tmp/evil.so")],
+            "kde-open supports separating its options from an unchanged path"
+        );
+    }
+
+    #[test]
+    fn leaves_urls_and_regular_paths_unchanged() {
+        for path in ["https://example.com", "document.pdf", "/tmp/-document.pdf"] {
+            let commands = commands(path);
+            let expected_len = 4;
+            let commands = &commands[commands.len() - expected_len..];
+            assert_eq!(args(&commands[0]), [OsStr::new(path)]);
+            assert_eq!(args(&commands[1]), [OsStr::new("open"), OsStr::new(path)]);
+            assert_eq!(args(&commands[2]), [OsStr::new(path)]);
+            assert_eq!(args(&commands[3]), [OsStr::new("--"), OsStr::new(path)]);
+        }
+    }
+}