Skip to content

mv,cp: fix xattr TOCTOU by using file descriptor-based operations #10545

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
sylvestre wants to merge 3 commits into
base: main
Choose a base branch
from
+220 −5
Draft

mv,cp: fix xattr TOCTOU by using file descriptor-based operations #10545

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e3b063d
mv,cp: fix xattr TOCTOU by using file descriptor-based operations
sylvestre Jan 28, 2026
2598f34
mv,cp: fix xattr and symlink handling in cross-device operations
sylvestre Jan 28, 2026
1eb4309
Merge branch 'main' into issue-10014
sylvestre Feb 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 47 additions & 5 deletions src/uu/mv/src/mv.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,8 @@ fn rename_dir_fallback(
(_, _) => None,
};

// Retrieve xattrs before copying (directories use path-based operations
// since they cannot be opened in write mode for xattr operations)
#[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))]
let xattrs = fsxattr::retrieve_xattrs(from).unwrap_or_else(|_| HashMap::new());

Expand All @@ -982,8 +984,14 @@ fn rename_dir_fallback(
display_manager,
);

// Apply xattrs after directory contents are copied, ignoring ENOTSUP errors
// (filesystem doesn't support xattrs, which is acceptable for cross-device moves)
#[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))]
fsxattr::apply_xattrs(to, xattrs)?;
if let Err(e) = fsxattr::apply_xattrs(to, xattrs) {
if e.raw_os_error() != Some(libc::EOPNOTSUPP) {
return Err(e);
}
}

result?;

Expand Down Expand Up @@ -1050,8 +1058,35 @@ fn copy_dir_contents_recursive(
pb.set_message(from_path.to_string_lossy().to_string());
}

if from_path.is_dir() {
// Recursively copy subdirectory
if from_path.is_symlink() {
// Handle symlinks first, before checking is_dir() which follows symlinks.
// This prevents symlinks to directories from being expanded into full copies.
#[cfg(unix)]
{
copy_file_with_hardlinks_helper(
&from_path,
&to_path,
hardlink_tracker,
hardlink_scanner,
)?;
}
#[cfg(not(unix))]
{
rename_symlink_fallback(&from_path, &to_path)?;
}

// Print verbose message for symlink
if verbose {
let message = translate!("mv-verbose-renamed", "from" => from_path.quote(), "to" => to_path.quote());
match display_manager {
Some(pb) => pb.suspend(|| {
println!("{message}");
}),
None => println!("{message}"),
}
}
} else if from_path.is_dir() {
// Recursively copy subdirectory (only real directories, not symlinks)
fs::create_dir_all(&to_path)?;

// Print verbose message for directory
Expand Down Expand Up @@ -1203,12 +1238,19 @@ fn rename_file_fallback(
Ok(())
}

/// Copy xattrs from source to destination, ignoring ENOTSUP/EOPNOTSUPP errors.
/// Copy xattrs from source to destination using file descriptors, ignoring ENOTSUP/EOPNOTSUPP errors.
/// This version avoids TOCTOU races by operating on file descriptors rather than paths.
/// These errors indicate the filesystem doesn't support extended attributes,
/// which is acceptable when moving files across filesystems.
#[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))]
fn copy_xattrs_if_supported(from: &Path, to: &Path) -> io::Result<()> {
match fsxattr::copy_xattrs(from, to) {
use std::fs::{File, OpenOptions};

// Open both files to pin their inodes, avoiding TOCTOU races during xattr operations
let source_file = File::open(from)?;
let dest_file = OpenOptions::new().write(true).open(to)?;

match fsxattr::copy_xattrs_fd(&source_file, &dest_file) {
Ok(()) => Ok(()),
Err(e) if e.raw_os_error() == Some(libc::EOPNOTSUPP) => Ok(()),
Err(e) => Err(e),
Expand Down
122 changes: 122 additions & 0 deletions src/uucore/src/lib/features/fsxattr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@
use itertools::Itertools;
use std::collections::HashMap;
use std::ffi::{OsStr, OsString};
use std::fs::File;
#[cfg(unix)]
use std::os::unix::ffi::OsStrExt;
use std::path::Path;
use xattr::FileExt;

/// Copies extended attributes (xattrs) from one file or directory to another.
///
Expand All @@ -32,6 +34,29 @@ pub fn copy_xattrs<P: AsRef<Path>>(source: P, dest: P) -> std::io::Result<()> {
Ok(())
}

/// Copies extended attributes (xattrs) from one file to another using file descriptors.
///
/// This version avoids TOCTOU (time-of-check to time-of-use) races by operating
/// on open file descriptors rather than paths, ensuring all operations target
/// the same inodes throughout.
///
/// # Arguments
///
/// * `source` - A reference to the source file (open file descriptor).
/// * `dest` - A reference to the destination file (open file descriptor).
///
/// # Returns
///
/// A result indicating success or failure.
pub fn copy_xattrs_fd(source: &File, dest: &File) -> std::io::Result<()> {
for attr_name in source.list_xattr()? {
if let Some(value) = source.get_xattr(&attr_name)? {
dest.set_xattr(&attr_name, &value)?;
}
}
Ok(())
}

/// Retrieves the extended attributes (xattrs) of a given file or directory.
///
/// # Arguments
Expand All @@ -51,6 +76,28 @@ pub fn retrieve_xattrs<P: AsRef<Path>>(source: P) -> std::io::Result<HashMap<OsS
Ok(attrs)
}

/// Retrieves the extended attributes (xattrs) of a given file using a file descriptor.
///
/// This version avoids TOCTOU races by operating on an open file descriptor
/// rather than a path, ensuring all operations target the same inode.
///
/// # Arguments
///
/// * `source` - A reference to the file (open file descriptor).
///
/// # Returns
///
/// A result containing a HashMap of attribute names and values, or an error.
pub fn retrieve_xattrs_fd(source: &File) -> std::io::Result<HashMap<OsString, Vec<u8>>> {
let mut attrs = HashMap::new();
for attr_name in source.list_xattr()? {
if let Some(value) = source.get_xattr(&attr_name)? {
attrs.insert(attr_name, value);
}
}
Ok(attrs)
}

/// Applies extended attributes (xattrs) to a given file or directory.
///
/// # Arguments
Expand All @@ -71,6 +118,26 @@ pub fn apply_xattrs<P: AsRef<Path>>(
Ok(())
}

/// Applies extended attributes (xattrs) to a given file using a file descriptor.
///
/// This version avoids TOCTOU races by operating on an open file descriptor
/// rather than a path, ensuring all operations target the same inode.
///
/// # Arguments
///
/// * `dest` - A reference to the file (open file descriptor).
/// * `xattrs` - A HashMap containing attribute names and their corresponding values.
///
/// # Returns
///
/// A result indicating success or failure.
pub fn apply_xattrs_fd(dest: &File, xattrs: HashMap<OsString, Vec<u8>>) -> std::io::Result<()> {
for (attr, value) in xattrs {
dest.set_xattr(&attr, &value)?;
}
Ok(())
}

/// Checks if a file has an Access Control List (ACL) based on its extended attributes.
///
/// # Arguments
Expand Down Expand Up @@ -286,4 +353,59 @@ mod tests {
assert!(has_security_cap_acl(&file_path));
}
}

#[test]
fn test_copy_xattrs_fd() {
use std::fs::OpenOptions;

let temp_dir = tempdir().unwrap();
let source_path = temp_dir.path().join("source.txt");
let dest_path = temp_dir.path().join("dest.txt");

File::create(&source_path).unwrap();
File::create(&dest_path).unwrap();

let test_attr = "user.test_fd";
let test_value = b"test value fd";
xattr::set(&source_path, test_attr, test_value).unwrap();

let source_file = File::open(&source_path).unwrap();
let dest_file = OpenOptions::new().write(true).open(&dest_path).unwrap();

copy_xattrs_fd(&source_file, &dest_file).unwrap();

let copied_value = xattr::get(&dest_path, test_attr).unwrap().unwrap();
assert_eq!(copied_value, test_value);
}

#[test]
fn test_apply_and_retrieve_xattrs_fd() {
use std::fs::OpenOptions;

let temp_dir = tempdir().unwrap();
let file_path = temp_dir.path().join("test_file.txt");

File::create(&file_path).unwrap();

let mut test_xattrs = HashMap::new();
let test_attr = "user.test_attr_fd";
let test_value = b"test value fd";
test_xattrs.insert(OsString::from(test_attr), test_value.to_vec());

// Apply using file descriptor
let file = OpenOptions::new().write(true).open(&file_path).unwrap();
apply_xattrs_fd(&file, test_xattrs).unwrap();
drop(file);

// Retrieve using file descriptor
let file = File::open(&file_path).unwrap();
let retrieved_xattrs = retrieve_xattrs_fd(&file).unwrap();
assert!(retrieved_xattrs.contains_key(OsString::from(test_attr).as_os_str()));
assert_eq!(
retrieved_xattrs
.get(OsString::from(test_attr).as_os_str())
.unwrap(),
test_value
);
}
}
51 changes: 51 additions & 0 deletions tests/by-util/test_mv.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2836,3 +2836,54 @@ fn test_mv_xattr_enotsup_silent() {
std::fs::remove_file("/dev/shm/mv_test").ok();
}
}

/// Test that symlinks inside directories are preserved during cross-device moves
/// (not expanded into full copies of their targets)
#[test]
#[cfg(target_os = "linux")]
fn test_mv_cross_device_symlink_preserved() {
use std::fs;
use std::os::unix::fs::symlink;
use tempfile::TempDir;

let scene = TestScenario::new(util_name!());
let at = &scene.fixtures;

// Create a directory with a symlink to /etc inside
at.mkdir("src_dir");
at.write("src_dir/local.txt", "local content");
symlink("/etc", at.plus("src_dir/etc_link")).expect("Failed to create symlink");

// Verify initial state
assert!(at.is_symlink("src_dir/etc_link"));

// Force cross-filesystem move using /dev/shm (tmpfs)
let target_dir =
TempDir::new_in("/dev/shm/").expect("Unable to create temp directory in /dev/shm");
let target_path = target_dir.path().join("dst_dir");

scene
.ucmd()
.arg("src_dir")
.arg(target_path.to_str().unwrap())
.succeeds()
.no_stderr();

// Verify source was removed
assert!(!at.dir_exists("src_dir"));

// Verify the symlink was preserved (not expanded)
let moved_symlink = target_path.join("etc_link");
assert!(
moved_symlink.is_symlink(),
"etc_link should still be a symlink after cross-device move"
);
assert_eq!(
fs::read_link(&moved_symlink).expect("Failed to read symlink"),
std::path::Path::new("/etc"),
"symlink should still point to /etc"
);

// Verify the regular file was also moved
assert!(target_path.join("local.txt").exists());
}