uutils/coreutils
0
0
Fork 0
mirror of https://github.com/uutils/coreutils.git synced 2026-06-10 15:48:22 -07:00
Code Issues Packages Projects Releases Wiki Activity

split: harden output open path against TOCTOU target swaps (#12565)

Browse Source 
Co-authored-by: can1357 <me@can.ac>
This commit is contained in:
Sylvestre Ledru
2026-06-05 18:09:05 +02:00
committed by GitHub
parent 6b22fda6b9
commit 58040feb5c
6 changed files with 222 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/workflows/CICD.yml
+1 -1
View File
@@ -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
src/uu/split/src/platform/mod.rs
+18 -3
View File
@@ -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<std::io::BufWriter<Box<dyn std::io::Write>>> {
// 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)
src/uu/split/src/platform/unix.rs
+99 -16
View File
@@ -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<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.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<dyn Write>))
}
@@ -169,6 +165,54 @@ pub fn instantiate_current_writer(
}
}
fn create_or_truncate_output_file(input: &OsStr, filename: &OsStr) -> 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: &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");
}
}
src/uu/split/src/platform/windows.rs
+59 -16
View File
@@ -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<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.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<dyn Write>))
}
fn create_or_truncate_output_file(input: &OsStr, filename: &OsStr) -> 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: &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)
src/uu/split/src/split.rs
+1 -1
View File
@@ -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)
}
}
util/check-safe-traversal.sh
+44 -1
View File
@@ -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.