fix: sanitize escape sequence injection

Author: curious-rabbit

src/error.rs

@@ -1,3 +1,12 @@
+use std::io::IsTerminal;
+
+use crate::sanitize::sanitize_for_terminal;
+
 pub fn print_error(msg: impl Into<String>) {
-    eprintln!("[fd error]: {}", msg.into());
+    let msg = msg.into();
+    if std::io::stderr().is_terminal() {
+        eprintln!("[fd error]: {}", sanitize_for_terminal(&msg));
+    } else {
+        eprintln!("[fd error]: {msg}");
+    }
 }

src/exec/mod.rs

@@ -63,9 +63,6 @@ impl CommandSet {
                     if cmd.number_of_tokens() > 1 {
                         bail!("Only one placeholder allowed for batch commands");
                     }
-                    if cmd.args[0].has_tokens() {
-                        bail!("First argument of exec-batch is expected to be a fixed executable");
-                    }
                     Ok(cmd)
                 })
                 .collect::<Result<Vec<_>>>()?,
@@ -178,7 +175,7 @@ impl CommandBuilder {
             self.finish()?;
         }
 
-        let arg = self.path_arg.generate(path, separator);
+        let arg = self.path_arg.generate_for_exec(path, separator);
         if !self
             .cmd
             .args_would_fit(iter::once(&arg).chain(&self.post_args))
@@ -245,6 +242,12 @@ impl CommandTemplate {
             bail!("No executable provided for --exec or --exec-batch");
         }
 
+        // The first argument is the command to execute and must be a fixed string;
+        // allowing a placeholder here would turn every matched file into an execve target.
+        if args[0].has_tokens() {
+            bail!("First argument of --exec/--exec-batch must be a fixed executable, not a placeholder");
+        }
+
         // If a placeholder token was not supplied, append one at the end of the command.
         if !has_placeholder {
             args.push(FormatTemplate::Tokens(vec![Token::Placeholder]));
@@ -262,9 +265,9 @@ impl CommandTemplate {
     /// Using the internal `args` field, and a supplied `input` variable, a `Command` will be
     /// build.
     fn generate(&self, input: &Path, path_separator: Option<&str>) -> io::Result<Command> {
-        let mut cmd = Command::new(self.args[0].generate(input, path_separator));
+        let mut cmd = Command::new(self.args[0].generate_for_exec(input, path_separator));
         for arg in &self.args[1..] {
-            cmd.try_arg(arg.generate(input, path_separator))?;
+            cmd.try_arg(arg.generate_for_exec(input, path_separator))?;
         }
         Ok(cmd)
     }
@@ -374,8 +377,8 @@ mod tests {
 
     #[test]
     fn tokens_with_literal_braces_and_placeholder() {
-        let template = CommandTemplate::new(vec!["{{{},end}"]).unwrap();
-        assert_eq!(generate_str(&template, "foo"), vec!["{foo,end}"]);
+        let template = CommandTemplate::new(vec!["echo", "{{{},end}"]).unwrap();
+        assert_eq!(generate_str(&template, "foo"), vec!["echo", "{foo,end}"]);
     }
 
     #[test]
@@ -429,6 +432,13 @@ mod tests {
         assert!(CommandSet::new(vec![vec!["echo"], vec![]]).is_err());
     }
 
+    #[test]
+    fn placeholder_as_executable_rejected() {
+        assert!(CommandSet::new(vec![vec!["{}"]]).is_err());
+        assert!(CommandSet::new(vec![vec!["{/}", "arg"]]).is_err());
+        assert!(CommandSet::new_batch(vec![vec!["{}"]]).is_err());
+    }
+
     #[test]
     fn generate_custom_path_separator() {
         let arg = FormatTemplate::Tokens(vec![Token::Placeholder]);

src/fmt/mod.rs

@@ -24,6 +24,19 @@ pub enum Token {
     Text(String),
 }
 
+impl Token {
+    fn is_path_derived(&self) -> bool {
+        matches!(
+            self,
+            Token::Placeholder
+                | Token::Basename
+                | Token::Parent
+                | Token::NoExt
+                | Token::BasenameNoExt
+        )
+    }
+}
+
 impl Display for Token {
     fn fmt(&self, f: &mut Formatter) -> fmt::Result {
         match *self {
@@ -110,8 +123,26 @@ impl FormatTemplate {
     /// the path separator in all placeholder tokens. Fixed text and tokens are not affected by
     /// path separator substitution.
     pub fn generate(&self, path: impl AsRef<Path>, path_separator: Option<&str>) -> OsString {
+        self.generate_impl(path.as_ref(), path_separator, false)
+    }
+
+    /// Like `generate`, but prepends `./` when a path-derived token produces a leading `-`,
+    /// so the target of `--exec` does not see it as an option.
+    pub fn generate_for_exec(
+        &self,
+        path: impl AsRef<Path>,
+        path_separator: Option<&str>,
+    ) -> OsString {
+        self.generate_impl(path.as_ref(), path_separator, true)
+    }
+
+    fn generate_impl(
+        &self,
+        path: &Path,
+        path_separator: Option<&str>,
+        protect_leading_dash: bool,
+    ) -> OsString {
         use Token::*;
-        let path = path.as_ref();
 
         match *self {
             Self::Tokens(ref tokens) => {
@@ -134,6 +165,16 @@ impl FormatTemplate {
                         Text(string) => s.push(string),
                     }
                 }
+
+                if protect_leading_dash
+                    && tokens.first().is_some_and(|t| t.is_path_derived())
+                    && s.as_encoded_bytes().first() == Some(&b'-')
+                {
+                    let mut prefixed = OsString::from("./");
+                    prefixed.push(s);
+                    return prefixed;
+                }
+
                 s
             }
             Self::Text(ref text) => OsString::from(text),
@@ -278,4 +319,86 @@ mod fmt_tests {
             basenameNoExt=file }"
         );
     }
+
+    fn tmpl(s: &str) -> FormatTemplate {
+        FormatTemplate::parse(s)
+    }
+
+    #[test]
+    fn exec_basename_with_leading_dash_is_guarded() {
+        let out = tmpl("{/}")
+            .generate_for_exec(Path::new("./some/dir/-rf"), None)
+            .into_string()
+            .unwrap();
+        assert_eq!(out, "./-rf");
+    }
+
+    #[test]
+    fn exec_basename_no_ext_with_leading_dash_is_guarded() {
+        let out = tmpl("{/.}")
+            .generate_for_exec(Path::new("./some/dir/-evil.txt"), None)
+            .into_string()
+            .unwrap();
+        assert_eq!(out, "./-evil");
+    }
+
+    #[test]
+    fn exec_parent_with_leading_dash_is_guarded() {
+        let out = tmpl("{//}")
+            .generate_for_exec(Path::new("-startdir/inner/file.txt"), None)
+            .into_string()
+            .unwrap();
+        assert_eq!(out, "./-startdir/inner");
+    }
+
+    #[test]
+    fn exec_plain_placeholder_already_safe() {
+        let out = tmpl("{}")
+            .generate_for_exec(Path::new("./some/dir/-rf"), None)
+            .into_string()
+            .unwrap();
+        assert_eq!(out, "./some/dir/-rf");
+    }
+
+    #[test]
+    fn exec_user_literal_dash_prefix_not_rewritten() {
+        let out = tmpl("-{/}")
+            .generate_for_exec(Path::new("./some/dir/normal.txt"), None)
+            .into_string()
+            .unwrap();
+        assert_eq!(out, "-normal.txt");
+    }
+
+    #[test]
+    fn exec_user_literal_dash_before_dash_basename() {
+        let out = tmpl("-{/}")
+            .generate_for_exec(Path::new("./some/dir/-name"), None)
+            .into_string()
+            .unwrap();
+        assert_eq!(out, "--name");
+    }
+
+    #[test]
+    fn exec_suffix_after_placeholder_preserves_guard() {
+        let out = tmpl("{/}.bak")
+            .generate_for_exec(Path::new("./some/dir/-foo"), None)
+            .into_string()
+            .unwrap();
+        assert_eq!(out, "./-foo.bak");
+    }
+
+    #[test]
+    fn format_mode_does_not_protect_leading_dash() {
+        let out = tmpl("{/}")
+            .generate(Path::new("./some/dir/-rf"), None)
+            .into_string()
+            .unwrap();
+        assert_eq!(out, "-rf");
+    }
+
+    #[test]
+    fn exec_empty_result_not_prefixed() {
+        let out = tmpl("{/}").generate_for_exec(Path::new(""), None);
+        assert!(out.is_empty());
+    }
 }

src/main.rs

@@ -11,6 +11,7 @@ mod fmt;
 mod hyperlink;
 mod output;
 mod regex_helper;
+mod sanitize;
 mod walk;
 
 use std::env;
@@ -65,7 +66,13 @@ fn main() {
             exit_code.exit();
         }
         Err(err) => {
-            eprintln!("[fd error]: {err:#}");
+            // Gap fix: anyhow errors bubbled from run() may embed user paths/args.
+            let formatted = format!("{err:#}");
+            if std::io::stderr().is_terminal() {
+                eprintln!("[fd error]: {}", sanitize::sanitize_for_terminal(&formatted));
+            } else {
+                eprintln!("[fd error]: {formatted}");
+            }
             ExitCode::GeneralError.exit();
         }
     }

src/output.rs

@@ -7,11 +7,21 @@ use crate::config::Config;
 use crate::dir_entry::DirEntry;
 use crate::fmt::FormatTemplate;
 use crate::hyperlink::PathUrl;
+use crate::sanitize::sanitize_for_terminal;
 
 fn replace_path_separator(path: &str, new_path_separator: &str) -> String {
     path.replace(std::path::MAIN_SEPARATOR, new_path_separator)
 }
 
+/// Sanitize a string for terminal output only; raw bytes pass through on pipes.
+fn maybe_sanitize<'a>(s: &'a str, config: &Config) -> Cow<'a, str> {
+    if config.interactive_terminal {
+        sanitize_for_terminal(s)
+    } else {
+        Cow::Borrowed(s)
+    }
+}
+
 // TODO: this function is performance critical and can probably be optimized
 pub fn print_entry<W: Write>(stdout: &mut W, entry: &DirEntry, config: &Config) -> io::Result<()> {
     let mut has_hyperlink = false;
@@ -76,7 +86,8 @@ fn print_entry_format<W: Write>(
         config.path_separator.as_deref(),
     );
     // TODO: support writing raw bytes on unix?
-    write!(stdout, "{}", output.to_string_lossy())
+    let s = output.to_string_lossy();
+    write!(stdout, "{}", maybe_sanitize(&s, config))
 }
 
 // TODO: this function is performance critical and can probably be optimized
@@ -86,7 +97,6 @@ fn print_entry_colorized<W: Write>(
     config: &Config,
     ls_colors: &LsColors,
 ) -> io::Result<()> {
-    // Split the path between the parent and the last component
     let mut offset = 0;
     let path = entry.stripped_path(config);
     let path_str = path.to_string_lossy();
@@ -112,14 +122,16 @@ fn print_entry_colorized<W: Write>(
             .style_for_indicator(Indicator::Directory)
             .map(Style::to_nu_ansi_term_style)
             .unwrap_or_default();
-        write!(stdout, "{}", style.paint(parent_str))?;
+        let safe_parent = maybe_sanitize(&parent_str, config);
+        write!(stdout, "{}", style.paint(safe_parent.as_ref()))?;
     }
 
     let style = entry
         .style(ls_colors)
         .map(Style::to_nu_ansi_term_style)
         .unwrap_or_default();
-    write!(stdout, "{}", style.paint(&path_str[offset..]))?;
+    let safe_basename = maybe_sanitize(&path_str[offset..], config);
+    write!(stdout, "{}", style.paint(safe_basename.as_ref()))?;
 
     print_trailing_slash(
         stdout,
@@ -143,7 +155,8 @@ fn print_entry_uncolorized_base<W: Write>(
     if let Some(ref separator) = config.path_separator {
         *path_string.to_mut() = replace_path_separator(&path_string, separator);
     }
-    write!(stdout, "{path_string}")?;
+    let safe = maybe_sanitize(&path_string, config);
+    write!(stdout, "{safe}")?;
     print_trailing_slash(stdout, entry, config, None)
 }
 
@@ -165,10 +178,9 @@ fn print_entry_uncolorized<W: Write>(
     use std::os::unix::ffi::OsStrExt;
 
     if config.interactive_terminal || config.path_separator.is_some() {
-        // Fall back to the base implementation
         print_entry_uncolorized_base(stdout, entry, config)
     } else {
-        // Print path as raw bytes, allowing invalid UTF-8 filenames to be passed to other processes
+        // Piped output: raw bytes so invalid UTF-8 filenames reach downstream tools intact.
         stdout.write_all(entry.stripped_path(config).as_os_str().as_bytes())?;
         print_trailing_slash(stdout, entry, config, None)
     }