split: harden output open path against TOCTOU target swaps

Author: can1357

src/uu/split/src/platform/unix.rs

@@ -8,6 +8,7 @@ use std::io::{BufWriter, Error, Result};
 use std::io::{ErrorKind, Write};
 use std::path::Path;
 use std::process::{Child, Command, Stdio};
+use uucore::display::Quotable;
 use uucore::error::USimpleError;
 use uucore::fs;
 use uucore::fs::FileInformation;
@@ -127,36 +128,32 @@ impl Drop for FilterWriter {
 /// Instantiate either a file writer or a "write to shell process's stdin" writer
 pub fn instantiate_current_writer(
     filter: Option<&str>,
+    input: &OsStr,
     filename: &str,
     is_new: bool,
 ) -> Result<BufWriter<Box<dyn Write>>> {
     match filter {
         None => {
             let file = if is_new {
-                // create new file
-                std::fs::OpenOptions::new()
-                    .write(true)
-                    .create(true)
-                    .truncate(true)
-                    .open(Path::new(&filename))
-                    .map_err(|e| match e.kind() {
-                        ErrorKind::IsADirectory => Error::other(
-                            translate!("split-error-is-a-directory", "dir" => filename),
-                        ),
-                        _ => Error::other(
-                            translate!("split-error-unable-to-open-file", "file" => filename),
-                        ),
-                    })?
+                create_or_truncate_output_file(input, filename)?
             } else {
                 // re-open file that we previously created to append to it
-                std::fs::OpenOptions::new()
+                let file = std::fs::OpenOptions::new()
                     .append(true)
                     .open(Path::new(&filename))
                     .map_err(|_| {
                         Error::other(
                             translate!("split-error-unable-to-reopen-file", "file" => filename),
                         )
-                    })?
+                    })?;
+
+                if input_and_output_refer_to_same_file(input, &file) {
+                    return Err(Error::other(
+                        translate!("split-error-would-overwrite-input", "file" => filename.quote()),
+                    ));
+                }
+
+                file
             };
             Ok(BufWriter::new(Box::new(file) as Box<dyn Write>))
         }
@@ -167,6 +164,52 @@ pub fn instantiate_current_writer(
     }
 }
 
+fn create_or_truncate_output_file(input: &OsStr, filename: &str) -> Result<std::fs::File> {
+    match std::fs::OpenOptions::new()
+        .write(true)
+        .create_new(true)
+        .open(Path::new(filename))
+    {
+        Ok(file) => Ok(file),
+        Err(e) if e.kind() == ErrorKind::AlreadyExists => {
+            let file = std::fs::OpenOptions::new()
+                .write(true)
+                .open(Path::new(filename))
+                .map_err(|err| open_file_error(filename, err.kind()))?;
+
+            if input_and_output_refer_to_same_file(input, &file) {
+                return Err(Error::other(
+                    translate!("split-error-would-overwrite-input", "file" => filename.quote()),
+                ));
+            }
+
+            file.set_len(0)
+                .map_err(|err| open_file_error(filename, err.kind()))?;
+            Ok(file)
+        }
+        Err(e) => Err(open_file_error(filename, e.kind())),
+    }
+}
+
+fn open_file_error(filename: &str, kind: ErrorKind) -> Error {
+    match kind {
+        ErrorKind::IsADirectory => {
+            Error::other(translate!("split-error-is-a-directory", "dir" => filename))
+        }
+        _ => Error::other(translate!("split-error-unable-to-open-file", "file" => filename)),
+    }
+}
+
+fn input_and_output_refer_to_same_file(input: &OsStr, output: &std::fs::File) -> bool {
+    let input_info = if input == "-" {
+        FileInformation::from_file(&std::io::stdin())
+    } else {
+        FileInformation::from_path(Path::new(input), true)
+    };
+
+    fs::infos_refer_to_same_file(input_info, FileInformation::from_file(output))
+}
+
 pub fn paths_refer_to_same_file(p1: &OsStr, p2: &OsStr) -> bool {
     // We have to take symlinks and relative paths into account.
     let p1 = if p1 == "-" {
@@ -176,3 +219,45 @@ pub fn paths_refer_to_same_file(p1: &OsStr, p2: &OsStr) -> bool {
     };
     fs::infos_refer_to_same_file(p1, FileInformation::from_path(Path::new(p2), true))
 }
+
+#[cfg(test)]
+mod tests {
+    use super::instantiate_current_writer;
+    use std::fs;
+    use std::os::unix::fs::symlink;
+    use std::time::{SystemTime, UNIX_EPOCH};
+
+    #[test]
+    fn reopened_writer_rejects_symlink_swapped_to_input() {
+        let tmp = std::env::temp_dir().join(format!(
+            "uutils-split-{}-{}",
+            std::process::id(),
+            SystemTime::now()
+                .duration_since(UNIX_EPOCH)
+                .expect("system time before unix epoch")
+                .as_nanos()
+        ));
+        fs::create_dir_all(&tmp).expect("failed to create temp dir");
+
+        let input = tmp.join("input.txt");
+        let output = tmp.join("xaa");
+        fs::write(&input, b"input\n").expect("failed to write input");
+        symlink(&input, &output).expect("failed to create output symlink");
+
+        let Err(err) = instantiate_current_writer(
+            None,
+            input.as_os_str(),
+            output.to_str().expect("non-utf8 path"),
+            false,
+        ) else {
+            panic!("re-opened writer should reject swapped symlink");
+        };
+        assert!(
+            err.to_string()
+                .contains("split-error-would-overwrite-input"),
+            "unexpected error: {err}"
+        );
+
+        fs::remove_dir_all(&tmp).expect("failed to cleanup temp dir");
+    }
+}

src/uu/split/src/platform/windows.rs

@@ -6,6 +6,7 @@ use std::ffi::OsStr;
 use std::io::{BufWriter, Error, Result};
 use std::io::{ErrorKind, Write};
 use std::path::Path;
+use uucore::display::Quotable;
 use uucore::fs;
 use uucore::translate;
 
@@ -15,36 +16,77 @@ use uucore::translate;
 /// a file writer
 pub fn instantiate_current_writer(
     _filter: Option<&str>,
+    input: &OsStr,
     filename: &str,
     is_new: bool,
 ) -> Result<BufWriter<Box<dyn Write>>> {
     let file = if is_new {
-        // create new file
-        std::fs::OpenOptions::new()
-            .write(true)
-            .create(true)
-            .truncate(true)
-            .open(Path::new(&filename))
-            .map_err(|e| match e.kind() {
-                ErrorKind::IsADirectory => {
-                    Error::other(translate!("split-error-is-a-directory", "dir" => filename))
-                }
-                _ => {
-                    Error::other(translate!("split-error-unable-to-open-file", "file" => filename))
-                }
-            })?
+        create_or_truncate_output_file(input, filename)?
     } else {
         // re-open file that we previously created to append to it
-        std::fs::OpenOptions::new()
+        let file = std::fs::OpenOptions::new()
             .append(true)
             .open(Path::new(&filename))
             .map_err(|_| {
                 Error::other(translate!("split-error-unable-to-reopen-file", "file" => filename))
-            })?
+            })?;
+
+        if input_and_output_refer_to_same_file(input, &file) {
+            return Err(Error::other(
+                translate!("split-error-would-overwrite-input", "file" => filename.quote()),
+            ));
+        }
+
+        file
     };
     Ok(BufWriter::new(Box::new(file) as Box<dyn Write>))
 }
 
+fn create_or_truncate_output_file(input: &OsStr, filename: &str) -> Result<std::fs::File> {
+    match std::fs::OpenOptions::new()
+        .write(true)
+        .create_new(true)
+        .open(Path::new(filename))
+    {
+        Ok(file) => Ok(file),
+        Err(e) if e.kind() == ErrorKind::AlreadyExists => {
+            let file = std::fs::OpenOptions::new()
+                .write(true)
+                .open(Path::new(filename))
+                .map_err(|err| open_file_error(filename, err.kind()))?;
+
+            if input_and_output_refer_to_same_file(input, &file) {
+                return Err(Error::other(
+                    translate!("split-error-would-overwrite-input", "file" => filename.quote()),
+                ));
+            }
+
+            file.set_len(0)
+                .map_err(|err| open_file_error(filename, err.kind()))?;
+            Ok(file)
+        }
+        Err(e) => Err(open_file_error(filename, e.kind())),
+    }
+}
+
+fn open_file_error(filename: &str, kind: ErrorKind) -> Error {
+    match kind {
+        ErrorKind::IsADirectory => {
+            Error::other(translate!("split-error-is-a-directory", "dir" => filename))
+        }
+        _ => Error::other(translate!("split-error-unable-to-open-file", "file" => filename)),
+    }
+}
+
+fn input_and_output_refer_to_same_file(input: &OsStr, output: &std::fs::File) -> bool {
+    let input_info = if input == "-" {
+        fs::FileInformation::from_file(&std::io::stdin())
+    } else {
+        fs::FileInformation::from_path(Path::new(input), true)
+    };
+
+    fs::infos_refer_to_same_file(input_info, fs::FileInformation::from_file(output))
+}
 pub fn paths_refer_to_same_file(p1: &OsStr, p2: &OsStr) -> bool {
     // Windows doesn't support many of the unix ways of paths being equals
     fs::paths_refer_to_same_file(Path::new(p1), Path::new(p2), true)

src/uu/split/src/split.rs

@@ -543,13 +543,15 @@ impl Settings {
         filename: &str,
         is_new: bool,
     ) -> io::Result<BufWriter<Box<dyn Write>>> {
-        if platform::paths_refer_to_same_file(&self.input, filename.as_ref()) {
+        if self.filter.is_some()
+            && platform::paths_refer_to_same_file(&self.input, filename.as_ref())
+        {
             return Err(io::Error::other(
                 translate!("split-error-would-overwrite-input", "file" => filename.quote()),
             ));
         }
 
-        platform::instantiate_current_writer(self.filter.as_deref(), filename, is_new)
+        platform::instantiate_current_writer(self.filter.as_deref(), &self.input, filename, is_new)
     }
 }