From 23b354d28af1c02d74fcf000b08c6b8a79fc6c27 Mon Sep 17 00:00:00 2001 From: can1357 Date: Wed, 18 Mar 2026 23:25:10 +0100 Subject: [PATCH] split: harden output open path against TOCTOU target swaps --- src/uu/split/src/platform/unix.rs | 117 +++++++++++++++++++++++---- src/uu/split/src/platform/windows.rs | 74 +++++++++++++---- src/uu/split/src/split.rs | 6 +- 3 files changed, 163 insertions(+), 34 deletions(-) diff --git a/src/uu/split/src/platform/unix.rs b/src/uu/split/src/platform/unix.rs index 656bd0109be..8350d77990f 100644 --- a/src/uu/split/src/platform/unix.rs +++ b/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>> { 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)) } @@ -167,6 +164,52 @@ pub fn instantiate_current_writer( } } +fn create_or_truncate_output_file(input: &OsStr, filename: &str) -> Result { + 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"); + } +} diff --git a/src/uu/split/src/platform/windows.rs b/src/uu/split/src/platform/windows.rs index 6693e4fe909..84bebcd9b2f 100644 --- a/src/uu/split/src/platform/windows.rs +++ b/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>> { 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)) } +fn create_or_truncate_output_file(input: &OsStr, filename: &str) -> Result { + 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) diff --git a/src/uu/split/src/split.rs b/src/uu/split/src/split.rs index 923f66ac134..509c3a5bde6 100644 --- a/src/uu/split/src/split.rs +++ b/src/uu/split/src/split.rs @@ -543,13 +543,15 @@ impl Settings { filename: &str, is_new: bool, ) -> io::Result>> { - 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) } }