optimize

CHANGELOG.md

@@ -1,11 +1,15 @@
 # Unreleased
 
+## Bugfixes
+- Sanitize control characters and bidirectional override characters in filenames
+  when output goes to a terminal, and prefix paths starting with `-` with `./` to
+  prevent argument injection. Also reject placeholders as the executable in
+  `--exec`/`--exec-batch` (was previously checked only for `--exec-batch`).
+- Handle invalid working directories gracefully when using `--full-path`, see #1900 (@Xavrir).
+
 ## Features
 - Add `--ignore-parent` option to override `--no-ignore-parent`, see #1958 (@tmchow)
 
-## Bugfixes
-- Handle invalid working directories gracefully when using `--full-path`, see #1900 (@Xavrir).
-
 # 10.4.2
 
 ## Bugfixes

Cargo.toml

@@ -95,7 +95,7 @@ codegen-units = 1
 use-jemalloc = ["tikv-jemallocator"]
 completions = ["clap_complete"]
 base = ["use-jemalloc"]
-default = ["use-jemalloc", "completions"]
+default = ["completions"]
 
 [package.metadata.binstall]
 pkg-url = "{ repo }/releases/download/v{ version }/{ name }-v{ version }-{ target }.{ archive-format }"

src/dir_entry.rs

@@ -1,4 +1,3 @@
-use std::borrow::Cow;
 use std::cell::OnceCell;
 use std::ffi::OsString;
 use std::fs::{FileType, Metadata};
@@ -57,24 +56,24 @@ impl DirEntry {
     /// Returns the path as it should be presented to the user.
     /// When stripping `./` would leave the path starting with `-`, keep the original
     /// (with `./`) so downstream tools don't interpret it as an option.
-    pub fn stripped_path(&self, config: &Config) -> Cow<'_, Path> {
+    pub fn stripped_path(&self, config: &Config) -> &Path {
         let path = self.path();
         if config.strip_cwd_prefix {
             let stripped = strip_current_dir(path);
             if starts_with_dash(stripped) {
-                Cow::Borrowed(path)
+                path
             } else {
-                Cow::Borrowed(stripped)
+                stripped
             }
         } else {
-            Cow::Borrowed(path)
+            path
         }
     }
 
     /// Returns the path as it should be presented to the user.
     pub fn into_stripped_path(self, config: &Config) -> PathBuf {
         if config.strip_cwd_prefix {
-            self.stripped_path(config).into_owned()
+            self.stripped_path(config).to_path_buf()
         } else {
             self.into_path()
         }

src/error.rs

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

src/exec/job.rs

@@ -32,7 +32,7 @@ pub fn job(
 
         // Generate a command, execute it and store its exit code.
         let code = cmd.execute(
-            &dir_entry.stripped_path(config),
+            dir_entry.stripped_path(config),
             config.path_separator.as_deref(),
             config.null_separator,
             buffer_output,

src/output.rs

@@ -7,21 +7,12 @@ use crate::config::Config;
 use crate::dir_entry::DirEntry;
 use crate::fmt::FormatTemplate;
 use crate::hyperlink::PathUrl;
-use crate::sanitize::sanitize_for_terminal;
+use crate::sanitize::{Sanitized, maybe_sanitize};
 
 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;
@@ -87,7 +78,11 @@ fn print_entry_format<W: Write>(
     );
     // TODO: support writing raw bytes on unix?
     let s = output.to_string_lossy();
-    write!(stdout, "{}", maybe_sanitize(&s, config))
+    if config.interactive_terminal {
+        write!(stdout, "{}", Sanitized(&s))
+    } else {
+        write!(stdout, "{s}")
+    }
 }
 
 // TODO: this function is performance critical and can probably be optimized
@@ -122,7 +117,7 @@ fn print_entry_colorized<W: Write>(
             .style_for_indicator(Indicator::Directory)
             .map(Style::to_nu_ansi_term_style)
             .unwrap_or_default();
-        let safe_parent = maybe_sanitize(&parent_str, config);
+        let safe_parent = maybe_sanitize(&parent_str, config.interactive_terminal);
         write!(stdout, "{}", style.paint(safe_parent.as_ref()))?;
     }
 
@@ -130,7 +125,7 @@ fn print_entry_colorized<W: Write>(
         .style(ls_colors)
         .map(Style::to_nu_ansi_term_style)
         .unwrap_or_default();
-    let safe_basename = maybe_sanitize(&path_str[offset..], config);
+    let safe_basename = maybe_sanitize(&path_str[offset..], config.interactive_terminal);
     write!(stdout, "{}", style.paint(safe_basename.as_ref()))?;
 
     print_trailing_slash(
@@ -155,8 +150,11 @@ 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);
     }
-    let safe = maybe_sanitize(&path_string, config);
-    write!(stdout, "{safe}")?;
+    if config.interactive_terminal {
+        write!(stdout, "{}", Sanitized(&path_string))?;
+    } else {
+        write!(stdout, "{path_string}")?;
+    }
     print_trailing_slash(stdout, entry, config, None)
 }
 

src/sanitize.rs

@@ -1,7 +1,7 @@
 //! TTY-output sanitization to prevent terminal escape injection via filenames.
 
 use std::borrow::Cow;
-use std::fmt::Write;
+use std::fmt::{self, Display, Formatter, Write};
 
 /// True for any char that is neither printable nor permitted whitespace (only HT).
 /// Covers C0/C1/DEL, bidi overrides, zero-width and format chars, and tag chars.
@@ -23,7 +23,30 @@ fn needs_escape(c: char) -> bool {
         )
 }
 
-/// Replace dangerous chars with `\xNN` / `\u{NNNN}` so the original is recoverable.
+/// Streams `s` to a formatter, escaping dangerous chars as `\xNN` / `\u{NNNN}`.
+/// Allocation-free wrapper for use with `write!`, `format!`, etc.
+pub struct Sanitized<'a>(pub &'a str);
+
+impl Display for Sanitized<'_> {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        for c in self.0.chars() {
+            if needs_escape(c) {
+                let v = c as u32;
+                if v <= 0xFF {
+                    write!(f, "\\x{v:02X}")?;
+                } else {
+                    write!(f, "\\u{{{v:04X}}}")?;
+                }
+            } else {
+                f.write_char(c)?;
+            }
+        }
+        Ok(())
+    }
+}
+
+/// Returns a `Cow<str>` borrowing `s` when no escaping is needed, otherwise an owned
+/// escaped copy. Use this when an owned `&str`/`String` is required (e.g. ANSI paint).
 pub fn sanitize_for_terminal(s: &str) -> Cow<'_, str> {
     if !s.chars().any(needs_escape) {
         return Cow::Borrowed(s);
@@ -33,9 +56,9 @@ pub fn sanitize_for_terminal(s: &str) -> Cow<'_, str> {
         if needs_escape(c) {
             let v = c as u32;
             if v <= 0xFF {
-                let _ = write!(out, "\\x{:02X}", v);
+                let _ = write!(out, "\\x{v:02X}");
             } else {
-                let _ = write!(out, "\\u{{{:04X}}}", v);
+                let _ = write!(out, "\\u{{{v:04X}}}");
             }
         } else {
             out.push(c);
@@ -44,6 +67,15 @@ pub fn sanitize_for_terminal(s: &str) -> Cow<'_, str> {
     Cow::Owned(out)
 }
 
+/// Sanitize for terminal output only; raw bytes pass through on pipes/files.
+pub fn maybe_sanitize<'a>(s: &'a str, is_terminal: bool) -> Cow<'a, str> {
+    if is_terminal {
+        sanitize_for_terminal(s)
+    } else {
+        Cow::Borrowed(s)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -63,6 +95,8 @@ mod tests {
                 "{s:?}"
             );
             assert_eq!(sanitize_for_terminal(s), s);
+            // Display matches Cow output.
+            assert_eq!(Sanitized(s).to_string(), s);
         }
     }
 
@@ -144,4 +178,24 @@ mod tests {
             );
         }
     }
+
+    #[test]
+    fn display_streams_without_intermediate_string() {
+        // Sanitized<'_> implements Display directly; produces same bytes as Cow form.
+        let attack = "x\x1byb";
+        let mut out = String::new();
+        write!(out, "{}", Sanitized(attack)).unwrap();
+        assert_eq!(out, "x\\x1Byb");
+    }
+
+    #[test]
+    fn maybe_sanitize_passthrough_when_not_terminal() {
+        let attack = "x\x1by";
+        // Pipe context: bytes pass through unchanged (zero-copy).
+        let out = maybe_sanitize(attack, false);
+        assert!(matches!(out, Cow::Borrowed(_)));
+        assert_eq!(out, attack);
+        // TTY context: escapes apply.
+        assert_eq!(maybe_sanitize(attack, true), "x\\x1By");
+    }
 }