diff --git a/src/uu/head/src/head.rs b/src/uu/head/src/head.rs index 3b5cbd0a9d4..83fc25eeb13 100644 --- a/src/uu/head/src/head.rs +++ b/src/uu/head/src/head.rs @@ -13,7 +13,7 @@ use std::io::{self, BufWriter, Read, Seek, SeekFrom, Write}; use std::num::TryFromIntError; #[cfg(unix)] use std::os::fd::AsFd; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use thiserror::Error; use uucore::display::{Quotable, print_verbatim}; use uucore::error::{FromIo, UError, UResult, USimpleError}; @@ -445,13 +445,14 @@ fn uu_head(options: &HeadOptions) -> UResult<()> { Ok(()) } else { - if Path::new(file).is_dir() { - show!(USimpleError::new( - 1, - translate!("head-error-reading-file", "name" => file.quote(), "err" => "Is a directory") - )); - continue; - } + // Open first, then check whether the fd refers to a + // directory. The previous `Path::is_dir()` check followed + // symlinks, and a subsequent `File::open` re-resolved the + // path — opening this race window: an attacker who flips + // `file` from regular file to symlink between the two + // syscalls would have the post-check open follow the + // swapped symlink (issue #11972). Checking on the open fd + // closes that window. let mut file_handle = match File::open(file) { Ok(f) => f, Err(err) => { @@ -461,6 +462,22 @@ fn uu_head(options: &HeadOptions) -> UResult<()> { continue; } }; + match file_handle.metadata() { + Ok(m) if m.is_dir() => { + show!(USimpleError::new( + 1, + translate!("head-error-reading-file", "name" => file.quote(), "err" => "Is a directory") + )); + continue; + } + Ok(_) => {} + Err(err) => { + show!(err.map_err_context( + || translate!("head-error-cannot-open", "name" => file.quote()) + )); + continue; + } + } if (options.files.len() > 1 && !options.quiet) || options.verbose { if !first { println!(); diff --git a/tests/by-util/test_head.rs b/tests/by-util/test_head.rs index 3024d4f8710..847099d43db 100644 --- a/tests/by-util/test_head.rs +++ b/tests/by-util/test_head.rs @@ -933,3 +933,44 @@ fn test_do_not_attempt_to_read_a_directory() { .fails_with_code(1) .stderr_contains("error reading '.'"); } + +/// Regression for #11972: head must reject directories detected on the +/// open fd, not via a separate `Path::is_dir()` call. A symlink that +/// resolves to a directory must still be rejected — verifying the fd +/// check survives an indirection. +#[test] +#[cfg(unix)] +fn test_head_rejects_directory_through_symlink() { + use std::os::unix::fs::symlink; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir("real_dir"); + symlink("real_dir", at.plus("link_to_dir")).unwrap(); + + scene + .ucmd() + .arg("link_to_dir") + .fails_with_code(1) + .stderr_contains("Is a directory"); +} + +/// Regression for #11972: a symlink that points to a regular file must +/// still be readable by head (the fd-based check must distinguish the +/// fd's mode, not the symlink's). +#[test] +#[cfg(unix)] +fn test_head_follows_symlink_to_regular_file() { + use std::os::unix::fs::symlink; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.write("regular", "hello\n"); + symlink("regular", at.plus("link_to_regular")).unwrap(); + + scene + .ucmd() + .arg("link_to_regular") + .succeeds() + .stdout_is("hello\n"); +}