diff --git a/Cargo.lock b/Cargo.lock index 6448e580a3d..41be847fe06 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3768,7 +3768,8 @@ version = "0.8.0" dependencies = [ "clap", "fluent", - "nix", + "libc", + "rustix", "uucore", ] diff --git a/src/uu/mkfifo/Cargo.toml b/src/uu/mkfifo/Cargo.toml index 63f1d8f4748..07c7b1e5dab 100644 --- a/src/uu/mkfifo/Cargo.toml +++ b/src/uu/mkfifo/Cargo.toml @@ -25,7 +25,10 @@ uucore = { workspace = true, features = ["fs", "mode"] } fluent = { workspace = true } [target.'cfg(unix)'.dependencies] -nix = { workspace = true, features = ["fs"] } +rustix = { workspace = true, features = ["fs", "process"] } + +[target.'cfg(target_vendor = "apple")'.dependencies] +libc = { workspace = true } [features] selinux = ["uucore/selinux"] diff --git a/src/uu/mkfifo/src/mkfifo.rs b/src/uu/mkfifo/src/mkfifo.rs index 648c94ee34c..af76c5cd8f3 100644 --- a/src/uu/mkfifo/src/mkfifo.rs +++ b/src/uu/mkfifo/src/mkfifo.rs @@ -4,10 +4,10 @@ // file that was distributed with this source code. use clap::{Arg, ArgAction, Command, value_parser}; -use nix::sys::stat::Mode; -use nix::unistd::mkfifo; +use rustix::fs::Mode; +use rustix::process::umask; +#[cfg(any(feature = "selinux", feature = "smack"))] use std::fs; -use std::os::unix::fs::PermissionsExt; use uucore::display::Quotable; use uucore::error::{UResult, USimpleError}; use uucore::translate; @@ -48,47 +48,48 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { }; for f in fifos { - if mkfifo(f.as_str(), Mode::from_bits_truncate(0o666)).is_err() { + // Clear umask around mkfifo so the kernel applies the exact + // requested mode atomically. Skipping the path-based chmod + // that used to follow this call closes the TOCTOU window an + // attacker could use to swap the FIFO for a symlink between + // mkfifo and chmod (issue #10020). + let prev_umask = umask(Mode::empty()); + let mkfifo_result = create_fifo(f.as_str(), mode); + umask(prev_umask); + + if mkfifo_result.is_err() { show!(USimpleError::new( 1, translate!("mkfifo-error-cannot-create-fifo", "path" => f.quote()), )); - continue; - } - - // Explicitly set the permissions to ignore umask - if let Err(e) = fs::set_permissions(&f, fs::Permissions::from_mode(mode)) { - return Err(USimpleError::new( - 1, - translate!("mkfifo-error-cannot-set-permissions", "path" => f.quote(), "error" => e), - )); - } - - // Apply SELinux context if requested - #[cfg(all(feature = "selinux", any(target_os = "linux", target_os = "android")))] - { - // Extract the SELinux related flags and options - let set_security_context = matches.get_flag(options::SECURITY_CONTEXT); - let context = matches.get_one::(options::CONTEXT); - - if set_security_context || context.is_some() { - use std::path::Path; - if let Err(e) = - uucore::selinux::set_selinux_security_context(Path::new(&f), context) - { - let _ = fs::remove_file(f); - return Err(USimpleError::new(1, e.to_string())); + } else { + // Apply SELinux context if requested + #[cfg(all(feature = "selinux", any(target_os = "linux", target_os = "android")))] + { + let set_security_context = matches.get_flag(options::SECURITY_CONTEXT); + let context = matches.get_one::(options::CONTEXT); + + if set_security_context || context.is_some() { + use std::path::Path; + if let Err(e) = + uucore::selinux::set_selinux_security_context(Path::new(&f), context) + { + let _ = fs::remove_file(f); + return Err(USimpleError::new(1, e.to_string())); + } } } - } - // Apply SMACK context if requested - #[cfg(all(feature = "smack", target_os = "linux"))] - { - let set_security_context = matches.get_flag(options::SECURITY_CONTEXT); - let context = matches.get_one::(options::CONTEXT); - if set_security_context || context.is_some() { - uucore::smack::set_smack_label_and_cleanup(&f, context, |p| fs::remove_file(p))?; + // Apply SMACK context if requested + #[cfg(all(feature = "smack", target_os = "linux"))] + { + let set_security_context = matches.get_flag(options::SECURITY_CONTEXT); + let context = matches.get_one::(options::CONTEXT); + if set_security_context || context.is_some() { + uucore::smack::set_smack_label_and_cleanup(&f, context, |p| { + fs::remove_file(p) + })?; + } } } } @@ -133,6 +134,25 @@ pub fn uu_app() -> Command { ) } +// `rustix::fs::mkfifoat` is unavailable on Apple targets, so fall back to +// libc's path-based `mkfifo` there. Both rely on the caller having cleared +// the umask so the requested mode is applied atomically (see issue #10020). +#[cfg(not(target_vendor = "apple"))] +fn create_fifo(path: &str, mode: u32) -> Result<(), ()> { + use rustix::fs::{CWD, mkfifoat}; + mkfifoat(CWD, path, Mode::from_bits_truncate(mode)).map_err(|_| ()) +} + +#[cfg(target_vendor = "apple")] +fn create_fifo(path: &str, mode: u32) -> Result<(), ()> { + use std::ffi::CString; + let c_path = CString::new(path).map_err(|_| ())?; + // SAFETY: `c_path` is a valid NUL-terminated C string and `mode` is a + // standard mode_t bit pattern. + let rc = unsafe { libc::mkfifo(c_path.as_ptr(), mode as libc::mode_t) }; + if rc == 0 { Ok(()) } else { Err(()) } +} + fn calculate_mode(mode_option: Option<&String>) -> Result { let umask = uucore::mode::get_umask(); let mode = 0o666; // Default mode for FIFOs diff --git a/util/check-toctou.sh b/util/check-toctou.sh new file mode 100755 index 00000000000..70ff6bdc69f --- /dev/null +++ b/util/check-toctou.sh @@ -0,0 +1,105 @@ +#!/bin/bash +# +# TOCTOU (time-of-check / time-of-use) verification. +# +# These strace-based checks assert that utilities do not split a +# security-sensitive operation across two path-based syscalls (e.g. a +# stat() before open() that an attacker can race). The companion +# script check-safe-traversal.sh covers a different concern: that +# recursive walkers use the openat() family rather than re-resolving +# multi-component paths during traversal. +# + +set -e + +: ${PROFILE:=release-small} +export PROFILE + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PROJECT_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" +TEMP_DIR=$(mktemp -d) + +fail_immediately() { + echo "❌ FAILED: $1" + echo "" + echo "Debug information available in: $TEMP_DIR/strace_*.log" + exit 1 +} + +cleanup() { + rm -rf "$TEMP_DIR" +} +trap cleanup EXIT + +echo "=== TOCTOU Verification ===" + +if [ -f "$PROJECT_ROOT/target/${PROFILE}/coreutils" ]; then + echo "Using multicall binary" + USE_MULTICALL=1 + COREUTILS_BIN="$PROJECT_ROOT/target/${PROFILE}/coreutils" +elif [ -f "$PROJECT_ROOT/target/${PROFILE}/mkfifo" ]; then + echo "Using individual binaries" + USE_MULTICALL=0 +else + echo "Error: No binaries found. Build first with 'cargo build --profile=${PROFILE}'" + exit 1 +fi + +cd "$TEMP_DIR" + +util_cmd() { + if [ "$USE_MULTICALL" -eq 1 ]; then + echo "$COREUTILS_BIN $1" + else + echo "$PROJECT_ROOT/target/${PROFILE}/$1" + fi +} + +if [ "$USE_MULTICALL" -eq 1 ]; then + AVAILABLE_UTILS=$($COREUTILS_BIN --list) +else + AVAILABLE_UTILS="" + for util in mkfifo; do + if [ -f "$PROJECT_ROOT/target/${PROFILE}/$util" ]; then + AVAILABLE_UTILS="$AVAILABLE_UTILS $util" + fi + done +fi + +# mkfifo must not call a path-based chmod after creating the FIFO: the +# second syscall would re-resolve the path and could be redirected by an +# attacker who swaps the FIFO for a symlink in between (issue #10020). +# After the fix, the kernel applies the requested mode atomically via +# mkfifo with cleared umask. +if echo "$AVAILABLE_UTILS" | grep -q "mkfifo"; then + mkfifo_cmd=$(util_cmd mkfifo) + rm -f test_fifo + # mkfifo(3)/mkfifoat(3) are libc wrappers; the underlying syscall + # is mknodat (or mknod on older kernels). Trace those plus any + # chmod variants. + strace -f -e trace=mknod,mknodat,chmod,fchmod,fchmodat,fchmodat2 \ + -o strace_mkfifo.log \ + $mkfifo_cmd -m 666 test_fifo 2>/dev/null || true + + if [ ! -s strace_mkfifo.log ]; then + fail_immediately "strace produced no output for mkfifo" + fi + if ! grep -qE 'mknodat?\(' strace_mkfifo.log; then + cat strace_mkfifo.log + fail_immediately "mkfifo must call mknod/mknodat to create the FIFO" + fi + + if grep -qE '\bchmod\([^,]*"test_fifo"' strace_mkfifo.log; then + cat strace_mkfifo.log + fail_immediately "mkfifo must not call path-based chmod after creation (issue #10020)" + fi + if grep -qE 'fchmodat2?\([^,]+, "test_fifo"' strace_mkfifo.log; then + cat strace_mkfifo.log + fail_immediately "mkfifo must not call fchmodat after creation (issue #10020)" + fi + echo "✓ mkfifo does not chmod after creation" + rm -f test_fifo +fi + +echo "" +echo "✓ TOCTOU verification completed"