diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index 0ca11b177b5..f22f3873399 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -903,7 +903,7 @@ jobs: - name: Install strace run: sudo apt-get update && sudo apt-get install -y strace - name: Build utilities with safe traversal - run: cargo build --profile=release-small -p uu_rm -p uu_chmod -p uu_chown -p uu_chgrp -p uu_mv -p uu_du + run: cargo build --profile=release-small -p uu_rm -p uu_chmod -p uu_chown -p uu_chgrp -p uu_mv -p uu_du -p uu_split - name: Run safe traversal verification run: ./util/check-safe-traversal.sh diff --git a/src/uu/split/src/platform/mod.rs b/src/uu/split/src/platform/mod.rs index 33dc874586e..c347b39b2eb 100644 --- a/src/uu/split/src/platform/mod.rs +++ b/src/uu/split/src/platform/mod.rs @@ -12,18 +12,33 @@ pub use self::windows::instantiate_current_writer; #[cfg(windows)] pub use self::windows::paths_refer_to_same_file; -// WASI: no process spawning (filter) or device/inode comparison. #[cfg(target_os = "wasi")] -pub fn paths_refer_to_same_file(_p1: &std::ffi::OsStr, _p2: &std::ffi::OsStr) -> bool { - false +use uucore::{display::Quotable, translate}; + +// WASI has no process spawning (the `--filter` writer) and no fd-based inode +// comparison, so it falls back to a path-based identity check via canonicalize. +#[cfg(target_os = "wasi")] +pub fn paths_refer_to_same_file(p1: &std::ffi::OsStr, p2: &std::ffi::OsStr) -> bool { + match (std::fs::canonicalize(p1), std::fs::canonicalize(p2)) { + (Ok(a), Ok(b)) => a == b, + _ => false, + } } #[cfg(target_os = "wasi")] pub fn instantiate_current_writer( _filter: Option<&str>, + input: &std::ffi::OsStr, filename: &std::ffi::OsStr, is_new: bool, ) -> std::io::Result>> { + // Refuse to truncate/overwrite the input. WASI cannot do the fd-based check + // unix/windows use, so this is a best-effort path comparison. + if paths_refer_to_same_file(input, filename) { + return Err(std::io::Error::other( + translate!("split-error-would-overwrite-input", "file" => filename.quote()), + )); + } let file = if is_new { std::fs::OpenOptions::new() .write(true) diff --git a/src/uu/split/src/platform/unix.rs b/src/uu/split/src/platform/unix.rs index c9e0964cc31..b3f13e1ba15 100644 --- a/src/uu/split/src/platform/unix.rs +++ b/src/uu/split/src/platform/unix.rs @@ -128,29 +128,17 @@ 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: &OsStr, 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.quote()), - ), - _ => Error::other( - translate!("split-error-unable-to-open-file", "file" => filename.quote()), - ), - })? + 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(|_| { @@ -158,7 +146,15 @@ pub fn instantiate_current_writer( "split-error-unable-to-reopen-file", "file" => filename.quote() )) - })? + })?; + + 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)) } @@ -169,6 +165,54 @@ pub fn instantiate_current_writer( } } +fn create_or_truncate_output_file(input: &OsStr, filename: &OsStr) -> 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: &OsStr, kind: ErrorKind) -> Error { + match kind { + ErrorKind::IsADirectory => { + Error::other(translate!("split-error-is-a-directory", "dir" => filename.quote())) + } + _ => { + Error::other(translate!("split-error-unable-to-open-file", "file" => filename.quote())) + } + } +} + +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 == "-" { @@ -178,3 +222,42 @@ 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.as_os_str(), 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 3efef2240b9..14ab602ab38 100644 --- a/src/uu/split/src/platform/windows.rs +++ b/src/uu/split/src/platform/windows.rs @@ -16,38 +16,81 @@ use uucore::translate; /// a file writer pub fn instantiate_current_writer( _filter: Option<&str>, + input: &OsStr, filename: &OsStr, 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.quote()), - ), - _ => Error::other( - translate!("split-error-unable-to-open-file", "file" => filename.quote()), - ), - })? + 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.quote()), ) - })? + })?; + + 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: &OsStr) -> 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: &OsStr, kind: ErrorKind) -> Error { + match kind { + ErrorKind::IsADirectory => { + Error::other(translate!("split-error-is-a-directory", "dir" => filename.quote())) + } + _ => { + Error::other(translate!("split-error-unable-to-open-file", "file" => filename.quote())) + } + } +} + +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 596e6ffb986..1e790a364d3 100644 --- a/src/uu/split/src/split.rs +++ b/src/uu/split/src/split.rs @@ -549,7 +549,7 @@ impl Settings { )); } - platform::instantiate_current_writer(self.filter.as_deref(), filename, is_new) + platform::instantiate_current_writer(self.filter.as_deref(), &self.input, filename, is_new) } } diff --git a/util/check-safe-traversal.sh b/util/check-safe-traversal.sh index 871abb0a9e9..f1842a9267f 100755 --- a/util/check-safe-traversal.sh +++ b/util/check-safe-traversal.sh @@ -191,7 +191,7 @@ if [ "$USE_MULTICALL" -eq 1 ]; then AVAILABLE_UTILS=$($COREUTILS_BIN --list) else AVAILABLE_UTILS="" - for util in rm chmod chown chgrp du mv cp; do + for util in rm chmod chown chgrp du mv cp split; do if [ -f "$PROJECT_ROOT/target/${PROFILE}/$util" ]; then AVAILABLE_UTILS="$AVAILABLE_UTILS $util" fi @@ -291,6 +291,49 @@ if echo "$AVAILABLE_UTILS" | grep -q "cp"; then rm -f test_cp_src test_cp_dst fi +# split must harden its output open against TOCTOU target swaps (issue #11401 / +# CVE-2026-35374). The old code opened the output by path with O_TRUNC, so an +# attacker could swap the just-validated output for a symlink and have split +# truncate a different file. The fix creates outputs atomically with +# O_CREAT|O_EXCL and only ever truncates via ftruncate after an fd-based check +# that the opened output is not the input -- so a split that would overwrite its +# own input is refused. +if echo "$AVAILABLE_UTILS" | grep -q "split"; then + if [ "$USE_MULTICALL" -eq 1 ]; then + split_cmd="$COREUTILS_BIN split" + else + split_cmd="$PROJECT_ROOT/target/${PROFILE}/split" + fi + + printf '0123456789abcdef' > split_input + strace -f -e trace=openat -o strace_split_output_open.log \ + $split_cmd -b 4 split_input split_out_ 2>/dev/null || true + + if ! grep -qE 'openat\(AT_FDCWD, "split_out_[a-z]+", [^)]*O_CREAT[^)]*O_EXCL' strace_split_output_open.log; then + cat strace_split_output_open.log + fail_immediately "split must create output files with O_CREAT|O_EXCL (issue #11401)" + fi + if grep -qE 'openat\(AT_FDCWD, "split_out_[a-z]+", [^)]*O_TRUNC' strace_split_output_open.log; then + cat strace_split_output_open.log + fail_immediately "split must not open output files with a path-based O_TRUNC (TOCTOU truncation risk, issue #11401)" + fi + echo "✓ split creates outputs with O_CREAT|O_EXCL and no path-based O_TRUNC" + rm -f split_input split_out_* + + # A split whose output already resolves (via a symlink) to the input must be + # refused, leaving the input untouched. + printf 'split_victim_payload' > split_victim + ln -s split_victim split_swap_aa + if $split_cmd -b 4 split_victim split_swap_ 2>/dev/null; then + fail_immediately "split must refuse when the output would overwrite the input (issue #11401)" + fi + if [ "$(cat split_victim)" != "split_victim_payload" ]; then + fail_immediately "split truncated its own input through a swapped output symlink (issue #11401)" + fi + echo "✓ split refuses to overwrite its input via a swapped output symlink" + rm -f split_victim split_swap_* +fi + # mv cross-device (EXDEV) must use fd-based *xattr ops (issue #10014). if echo "$AVAILABLE_UTILS" | grep -q "mv" && [ -d /dev/shm ]; then # Need different filesystems for the EXDEV fallback to fire.