fsxattr: use fd-based xattr operations to prevent TOCTOU

Author: sylvestre

src/uucore/src/lib/features/fsxattr.rs

@@ -3,7 +3,7 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
-// spell-checker:ignore getxattr posix_acl_default
+// spell-checker:ignore getxattr posix_acl_default flistxattr fgetxattr fsetxattr
 
 //! Set of functions to manage xattr on files and dirs
 use itertools::Itertools;
@@ -15,6 +15,10 @@ use std::path::Path;
 
 /// Copies extended attributes (xattrs) from one file or directory to another.
 ///
+/// This function uses file descriptor-based operations to avoid TOCTOU vulnerabilities.
+/// By opening file descriptors first, the inodes are pinned, preventing the paths from
+/// being manipulated to point to different files during the xattr copy operation.
+///
 /// # Arguments
 ///
 /// * `source` - A reference to the source path.
@@ -23,7 +27,69 @@ use std::path::Path;
 /// # Returns
 ///
 /// A result indicating success or failure.
+///
+/// # Security
+///
+/// This implementation prevents TOCTOU attacks by using file descriptor-based xattr
+/// operations (flistxattr, fgetxattr, fsetxattr) instead of path-based operations.
+#[cfg(unix)]
 pub fn copy_xattrs<P: AsRef<Path>>(source: P, dest: P) -> std::io::Result<()> {
+    use nix::fcntl::{OFlag, open};
+    use nix::sys::stat::Mode;
+    use std::fs::{File, OpenOptions};
+    use xattr::FileExt;
+
+    let source_path = source.as_ref();
+    let dest_path = dest.as_ref();
+
+    // Check file types to determine how to open them
+    let source_meta = source_path.symlink_metadata()?;
+    let dest_meta = dest_path.symlink_metadata()?;
+
+    // Open file descriptors to pin the inodes and prevent TOCTOU
+    let (source_file, dest_file) = if source_meta.is_symlink() || dest_meta.is_symlink() {
+        // For symlinks, use O_PATH | O_NOFOLLOW to open without following
+        // O_PATH allows opening symlinks and getting an fd for xattr operations
+        let source_fd = open(
+            source_path,
+            OFlag::O_PATH | OFlag::O_NOFOLLOW,
+            Mode::empty(),
+        )
+        .map_err(|e| std::io::Error::from_raw_os_error(e as i32))?;
+
+        let dest_fd = open(dest_path, OFlag::O_PATH | OFlag::O_NOFOLLOW, Mode::empty())
+            .map_err(|e| std::io::Error::from_raw_os_error(e as i32))?;
+
+        (File::from(source_fd), File::from(dest_fd))
+    } else {
+        // For regular files and directories, open normally
+        let source_file = File::open(source_path)?;
+
+        let dest_file = if dest_meta.is_dir() {
+            // For directories, open with read-only (can't open directories with O_RDWR)
+            File::open(dest_path)?
+        } else {
+            // For regular files, open with read+write
+            OpenOptions::new().read(true).write(true).open(dest_path)?
+        };
+
+        (source_file, dest_file)
+    };
+
+    // Use fd-based xattr operations via FileExt trait
+    // These operations use the pinned file descriptors, preventing TOCTOU
+    for attr_name in source_file.list_xattr()? {
+        if let Some(value) = source_file.get_xattr(&attr_name)? {
+            dest_file.set_xattr(&attr_name, &value)?;
+        }
+    }
+
+    Ok(())
+}
+
+#[cfg(not(unix))]
+pub fn copy_xattrs<P: AsRef<Path>>(source: P, dest: P) -> std::io::Result<()> {
+    // Non-Unix platforms: fall back to path-based operations
     for attr_name in xattr::list(&source)? {
         if let Some(value) = xattr::get(&source, &attr_name)? {
             xattr::set(&dest, &attr_name, &value)?;
@@ -34,14 +100,58 @@ pub fn copy_xattrs<P: AsRef<Path>>(source: P, dest: P) -> std::io::Result<()> {
 
 /// Retrieves the extended attributes (xattrs) of a given file or directory.
 ///
+/// This function uses file descriptor-based operations to avoid TOCTOU vulnerabilities.
+///
 /// # Arguments
 ///
 /// * `source` - A reference to the path of the file or directory.
 ///
 /// # Returns
 ///
 /// A result containing a HashMap of attributes names and values, or an error.
+///
+/// # Security
+///
+/// This implementation prevents TOCTOU attacks by opening a file descriptor once
+/// and using it for all xattr operations, ensuring all operations act on the same inode.
+#[cfg(unix)]
 pub fn retrieve_xattrs<P: AsRef<Path>>(source: P) -> std::io::Result<HashMap<OsString, Vec<u8>>> {
+    use nix::fcntl::{OFlag, open};
+    use nix::sys::stat::Mode;
+    use std::fs::File;
+    use xattr::FileExt;
+
+    let source_path = source.as_ref();
+    let source_meta = source_path.symlink_metadata()?;
+
+    // Open file descriptor to pin the inode and prevent TOCTOU
+    let source_file = if source_meta.is_symlink() {
+        // For symlinks, use O_PATH | O_NOFOLLOW
+        let source_fd = open(
+            source_path,
+            OFlag::O_PATH | OFlag::O_NOFOLLOW,
+            Mode::empty(),
+        )
+        .map_err(|e| std::io::Error::from_raw_os_error(e as i32))?;
+        File::from(source_fd)
+    } else {
+        // For regular files and directories
+        File::open(source_path)?
+    };
+
+    // Use fd-based operations
+    let mut attrs = HashMap::new();
+    for attr_name in source_file.list_xattr()? {
+        if let Some(value) = source_file.get_xattr(&attr_name)? {
+            attrs.insert(attr_name, value);
+        }
+    }
+    Ok(attrs)
+}
+
+#[cfg(not(unix))]
+pub fn retrieve_xattrs<P: AsRef<Path>>(source: P) -> std::io::Result<HashMap<OsString, Vec<u8>>> {
+    // Non-Unix platforms: fall back to path-based operations
     let mut attrs = HashMap::new();
     for attr_name in xattr::list(&source)? {
         if let Some(value) = xattr::get(&source, &attr_name)? {
@@ -53,6 +163,8 @@ pub fn retrieve_xattrs<P: AsRef<Path>>(source: P) -> std::io::Result<HashMap<OsS
 
 /// Applies extended attributes (xattrs) to a given file or directory.
 ///
+/// This function uses file descriptor-based operations to avoid TOCTOU vulnerabilities.
+///
 /// # Arguments
 ///
 /// * `dest` - A reference to the path of the file or directory.
@@ -61,10 +173,52 @@ pub fn retrieve_xattrs<P: AsRef<Path>>(source: P) -> std::io::Result<HashMap<OsS
 /// # Returns
 ///
 /// A result indicating success or failure.
+///
+/// # Security
+///
+/// This implementation prevents TOCTOU attacks by opening a file descriptor once
+/// and using it for all xattr operations, ensuring all operations act on the same inode.
+#[cfg(unix)]
+pub fn apply_xattrs<P: AsRef<Path>>(
+    dest: P,
+    xattrs: HashMap<OsString, Vec<u8>>,
+) -> std::io::Result<()> {
+    use nix::fcntl::{OFlag, open};
+    use nix::sys::stat::Mode;
+    use std::fs::{File, OpenOptions};
+    use xattr::FileExt;
+
+    let dest_path = dest.as_ref();
+    let dest_meta = dest_path.symlink_metadata()?;
+
+    // Open file descriptor to pin the inode and prevent TOCTOU
+    let dest_file = if dest_meta.is_symlink() {
+        // For symlinks, use O_PATH | O_NOFOLLOW
+        let dest_fd = open(dest_path, OFlag::O_PATH | OFlag::O_NOFOLLOW, Mode::empty())
+            .map_err(|e| std::io::Error::from_raw_os_error(e as i32))?;
+        File::from(dest_fd)
+    } else if dest_meta.is_dir() {
+        // For directories, open with read-only (can't open directories with O_RDWR)
+        // Setting xattrs works with O_RDONLY if we have write permissions on the directory
+        File::open(dest_path)?
+    } else {
+        // For regular files, open with read+write
+        OpenOptions::new().read(true).write(true).open(dest_path)?
+    };
+
+    // Use fd-based operations
+    for (attr, value) in xattrs {
+        dest_file.set_xattr(&attr, &value)?;
+    }
+    Ok(())
+}
+
+#[cfg(not(unix))]
 pub fn apply_xattrs<P: AsRef<Path>>(
     dest: P,
     xattrs: HashMap<OsString, Vec<u8>>,
 ) -> std::io::Result<()> {
+    // Non-Unix platforms: fall back to path-based operations
     for (attr, value) in xattrs {
         xattr::set(&dest, &attr, &value)?;
     }