Fix CVE-2022-21658 for UNIX-like

Author: hkratz

library/std/src/fs/tests.rs

@@ -4,8 +4,10 @@ use crate::fs::{self, File, OpenOptions};
 use crate::io::{ErrorKind, SeekFrom};
 use crate::path::Path;
 use crate::str;
+use crate::sync::Arc;
 use crate::sys_common::io::test::{tmpdir, TempDir};
 use crate::thread;
+use crate::time::{Duration, Instant};
 
 use rand::{rngs::StdRng, RngCore, SeedableRng};
 
@@ -601,6 +603,21 @@ fn recursive_rmdir_of_symlink() {
     assert!(canary.exists());
 }
 
+#[test]
+fn recursive_rmdir_of_file_fails() {
+    // test we do not delete a directly specified file.
+    let tmpdir = tmpdir();
+    let canary = tmpdir.join("do_not_delete");
+    check!(check!(File::create(&canary)).write(b"foo"));
+    let result = fs::remove_dir_all(&canary);
+    #[cfg(unix)]
+    error!(result, "Not a directory");
+    #[cfg(windows)]
+    error!(result, 267); // ERROR_DIRECTORY - The directory name is invalid.
+    assert!(result.is_err());
+    assert!(canary.exists());
+}
+
 #[test]
 // only Windows makes a distinction between file and directory symlinks.
 #[cfg(windows)]
@@ -620,6 +637,59 @@ fn recursive_rmdir_of_file_symlink() {
     }
 }
 
+#[test]
+#[ignore] // takes too much time
+fn recursive_rmdir_toctou() {
+    // Test for time-of-check to time-of-use issues.
+    //
+    // Scenario:
+    // The attacker wants to get directory contents deleted, to which he does not have access.
+    // He has a way to get a privileged Rust binary call `std::fs::remove_dir_all()` on a
+    // directory he controls, e.g. in his home directory.
+    //
+    // The POC sets up the `attack_dest/attack_file` which the attacker wants to have deleted.
+    // The attacker repeatedly creates a directory and replaces it with a symlink from
+    // `victim_del` to `attack_dest` while the victim code calls `std::fs::remove_dir_all()`
+    // on `victim_del`. After a few seconds the attack has succeeded and
+    // `attack_dest/attack_file` is deleted.
+    let tmpdir = tmpdir();
+    let victim_del_path = tmpdir.join("victim_del");
+    let victim_del_path_clone = victim_del_path.clone();
+
+    // setup dest
+    let attack_dest_dir = tmpdir.join("attack_dest");
+    let attack_dest_dir = attack_dest_dir.as_path();
+    fs::create_dir(attack_dest_dir).unwrap();
+    let attack_dest_file = tmpdir.join("attack_dest/attack_file");
+    File::create(&attack_dest_file).unwrap();
+
+    let drop_canary_arc = Arc::new(());
+    let drop_canary_weak = Arc::downgrade(&drop_canary_arc);
+
+    eprintln!("x: {:?}", &victim_del_path);
+
+    // victim just continuously removes `victim_del`
+    thread::spawn(move || {
+        while drop_canary_weak.upgrade().is_some() {
+            let _ = fs::remove_dir_all(&victim_del_path_clone);
+        }
+    });
+
+    // attacker (could of course be in a separate process)
+    let start_time = Instant::now();
+    while Instant::now().duration_since(start_time) < Duration::from_secs(1000) {
+        if !attack_dest_file.exists() {
+            panic!(
+                "Victim deleted symlinked file outside of victim_del. Attack succeeded in {:?}.",
+                Instant::now().duration_since(start_time)
+            );
+        }
+        let _ = fs::create_dir(&victim_del_path);
+        let _ = fs::remove_dir(&victim_del_path);
+        let _ = symlink_dir(attack_dest_dir, &victim_del_path);
+    }
+}
+
 #[test]
 fn unicode_path_is_dir() {
     assert!(Path::new(".").is_dir());

library/std/src/sys/unix/fs.rs

@@ -64,7 +64,7 @@ use libc::{
     dirent64, fstat64, ftruncate64, lseek64, lstat64, off64_t, open64, readdir64_r, stat64,
 };
 
-pub use crate::sys_common::fs::{remove_dir_all, try_exists};
+pub use crate::sys_common::fs::try_exists;
 
 pub struct File(FileDesc);
 
@@ -228,7 +228,7 @@ pub struct DirEntry {
         target_os = "fuchsia",
         target_os = "redox"
     ))]
-    name: Box<[u8]>,
+    name: CString,
 }
 
 #[derive(Clone, Debug)]
@@ -455,8 +455,6 @@ impl Iterator for ReadDir {
         target_os = "illumos"
     ))]
     fn next(&mut self) -> Option<io::Result<DirEntry>> {
-        use crate::slice;
-
         unsafe {
             loop {
                 // Although readdir_r(3) would be a correct function to use here because
@@ -474,14 +472,10 @@ impl Iterator for ReadDir {
                     };
                 }
 
-                let name = (*entry_ptr).d_name.as_ptr();
-                let namelen = libc::strlen(name) as usize;
-
                 let ret = DirEntry {
                     entry: *entry_ptr,
-                    name: slice::from_raw_parts(name as *const u8, namelen as usize)
-                        .to_owned()
-                        .into_boxed_slice(),
+                    // d_name is guaranteed to be null-terminated.
+                    name: CStr::from_ptr((*entry_ptr).d_name.as_ptr()).to_owned(),
                     dir: Arc::clone(&self.inner),
                 };
                 if ret.name_bytes() != b"." && ret.name_bytes() != b".." {
@@ -664,7 +658,26 @@ impl DirEntry {
         target_os = "redox"
     ))]
     fn name_bytes(&self) -> &[u8] {
-        &*self.name
+        self.name.as_bytes()
+    }
+
+    #[cfg(not(any(
+        target_os = "solaris",
+        target_os = "illumos",
+        target_os = "fuchsia",
+        target_os = "redox"
+    )))]
+    fn name_cstr(&self) -> &CStr {
+        unsafe { CStr::from_ptr(self.entry.d_name.as_ptr()) }
+    }
+    #[cfg(any(
+        target_os = "solaris",
+        target_os = "illumos",
+        target_os = "fuchsia",
+        target_os = "redox"
+    ))]
+    fn name_cstr(&self) -> &CStr {
+        &self.name
     }
 
     pub fn file_name_os_str(&self) -> &OsStr {
@@ -1437,3 +1450,256 @@ pub fn chroot(dir: &Path) -> io::Result<()> {
     cvt(unsafe { libc::chroot(dir.as_ptr()) })?;
     Ok(())
 }
+
+pub use remove_dir_impl::remove_dir_all;
+
+// Fallback for REDOX
+#[cfg(target_os = "redox")]
+mod remove_dir_impl {
+    pub use crate::sys_common::fs::remove_dir_all;
+}
+
+// Dynamically choose implementation Macos x86-64: modern for 10.10+, fallback for older versions
+#[cfg(all(target_os = "macos", target_arch = "x86_64"))]
+mod remove_dir_impl {
+    use super::{cstr, lstat, Dir, InnerReadDir, ReadDir};
+    use crate::ffi::CStr;
+    use crate::io;
+    use crate::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd};
+    use crate::os::unix::prelude::{OwnedFd, RawFd};
+    use crate::path::{Path, PathBuf};
+    use crate::sync::Arc;
+    use crate::sys::weak::weak;
+    use crate::sys::{cvt, cvt_r};
+    use libc::{c_char, c_int, DIR};
+
+    pub fn openat_nofollow_dironly(parent_fd: Option<RawFd>, p: &CStr) -> io::Result<OwnedFd> {
+        weak!(fn openat(c_int, *const c_char, c_int) -> c_int);
+        let fd = cvt_r(|| unsafe {
+            openat.get().unwrap()(
+                parent_fd.unwrap_or(libc::AT_FDCWD),
+                p.as_ptr(),
+                libc::O_CLOEXEC | libc::O_RDONLY | libc::O_NOFOLLOW | libc::O_DIRECTORY,
+            )
+        })?;
+        Ok(unsafe { OwnedFd::from_raw_fd(fd) })
+    }
+
+    fn fdreaddir(dir_fd: OwnedFd) -> io::Result<(ReadDir, RawFd)> {
+        weak!(fn fdopendir(c_int) -> *mut DIR, "fdopendir$INODE64");
+        let ptr = unsafe { fdopendir.get().unwrap()(dir_fd.as_raw_fd()) };
+        if ptr.is_null() {
+            return Err(io::Error::last_os_error());
+        }
+        let dirp = Dir(ptr);
+        // file descriptor is automatically closed by libc::closedir() now, so give up ownership
+        let new_parent_fd = dir_fd.into_raw_fd();
+        // a valid root is not needed because we do not call any functions involving the full path
+        // of the DirEntrys.
+        let dummy_root = PathBuf::new();
+        Ok((
+            ReadDir {
+                inner: Arc::new(InnerReadDir { dirp, root: dummy_root }),
+                end_of_stream: false,
+            },
+            new_parent_fd,
+        ))
+    }
+
+    fn remove_dir_all_recursive(parent_fd: Option<RawFd>, p: &Path) -> io::Result<()> {
+        weak!(fn unlinkat(c_int, *const c_char, c_int) -> c_int);
+
+        let pcstr = cstr(p)?;
+
+        // entry is expected to be a directory, open as such
+        let fd = openat_nofollow_dironly(parent_fd, &pcstr)?;
+
+        // open the directory passing ownership of the fd
+        let (dir, fd) = fdreaddir(fd)?;
+        for child in dir {
+            let child = child?;
+            match child.entry.d_type {
+                libc::DT_DIR => {
+                    remove_dir_all_recursive(Some(fd), Path::new(&child.file_name()))?;
+                }
+                libc::DT_UNKNOWN => {
+                    match cvt(unsafe { unlinkat.get().unwrap()(fd, child.name_cstr().as_ptr(), 0) })
+                    {
+                        // type unknown - try to unlink
+                        Err(err) if err.raw_os_error() == Some(libc::EPERM) => {
+                            // if the file is a directory unlink fails with EPERM
+                            remove_dir_all_recursive(Some(fd), Path::new(&child.file_name()))?;
+                        }
+                        result => {
+                            result?;
+                        }
+                    }
+                }
+                _ => {
+                    // not a directory -> unlink
+                    cvt(unsafe { unlinkat.get().unwrap()(fd, child.name_cstr().as_ptr(), 0) })?;
+                }
+            }
+        }
+
+        // unlink the directory after removing its contents
+        cvt(unsafe {
+            unlinkat.get().unwrap()(
+                parent_fd.unwrap_or(libc::AT_FDCWD),
+                pcstr.as_ptr(),
+                libc::AT_REMOVEDIR,
+            )
+        })?;
+        Ok(())
+    }
+
+    fn remove_dir_all_modern(p: &Path) -> io::Result<()> {
+        // We cannot just call remove_dir_all_recursive() here because that would not delete a passed
+        // symlink. No need to worry about races, because remove_dir_all_recursive() does not recurse
+        // into symlinks.
+        let attr = lstat(p)?;
+        if attr.file_type().is_symlink() {
+            crate::fs::remove_file(p)
+        } else {
+            remove_dir_all_recursive(None, p)
+        }
+    }
+
+    pub fn remove_dir_all(p: &Path) -> io::Result<()> {
+        weak!(fn openat(c_int, *const c_char, c_int) -> c_int);
+        if openat.get().is_some() {
+            // openat() is available with macOS 10.10+, just like unlinkat() and fdopendir()
+            remove_dir_all_modern(p)
+        } else {
+            // fall back to classic implementation
+            crate::sys_common::fs::remove_dir_all(p)
+        }
+    }
+}
+
+// Modern implementation using openat(), unlinkat() and fdopendir()
+#[cfg(not(any(all(target_os = "macos", target_arch = "x86_64"), target_os = "redox")))]
+mod remove_dir_impl {
+    use super::{cstr, lstat, Dir, DirEntry, InnerReadDir, ReadDir};
+    use crate::ffi::CStr;
+    use crate::io;
+    use crate::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd};
+    use crate::os::unix::prelude::{OwnedFd, RawFd};
+    use crate::path::{Path, PathBuf};
+    use crate::sync::Arc;
+    use crate::sys::{cvt, cvt_r};
+    use libc::{fdopendir, openat, unlinkat};
+
+    pub fn openat_nofollow_dironly(parent_fd: Option<RawFd>, p: &CStr) -> io::Result<OwnedFd> {
+        let fd = cvt_r(|| unsafe {
+            openat(
+                parent_fd.unwrap_or(libc::AT_FDCWD),
+                p.as_ptr(),
+                libc::O_CLOEXEC | libc::O_RDONLY | libc::O_NOFOLLOW | libc::O_DIRECTORY,
+            )
+        })?;
+        Ok(unsafe { OwnedFd::from_raw_fd(fd) })
+    }
+
+    fn fdreaddir(dir_fd: OwnedFd) -> io::Result<(ReadDir, RawFd)> {
+        let ptr = unsafe { fdopendir(dir_fd.as_raw_fd()) };
+        if ptr.is_null() {
+            return Err(io::Error::last_os_error());
+        }
+        let dirp = Dir(ptr);
+        // file descriptor is automatically closed by libc::closedir() now, so give up ownership
+        let new_parent_fd = dir_fd.into_raw_fd();
+        // a valid root is not needed because we do not call any functions involving the full path
+        // of the DirEntrys.
+        let dummy_root = PathBuf::new();
+        Ok((
+            ReadDir {
+                inner: Arc::new(InnerReadDir { dirp, root: dummy_root }),
+                #[cfg(not(any(
+                    target_os = "solaris",
+                    target_os = "illumos",
+                    target_os = "fuchsia",
+                    target_os = "redox",
+                )))]
+                end_of_stream: false,
+            },
+            new_parent_fd,
+        ))
+    }
+
+    #[cfg(any(
+        target_os = "solaris",
+        target_os = "illumos",
+        target_os = "haiku",
+        target_os = "vxworks"
+    ))]
+    fn is_dir(_ent: &DirEntry) -> Option<bool> {
+        None
+    }
+
+    #[cfg(not(any(
+        target_os = "solaris",
+        target_os = "illumos",
+        target_os = "haiku",
+        target_os = "vxworks"
+    )))]
+    fn is_dir(ent: &DirEntry) -> Option<bool> {
+        match ent.entry.d_type {
+            libc::DT_UNKNOWN => None,
+            libc::DT_DIR => Some(true),
+            _ => Some(false),
+        }
+    }
+
+    fn remove_dir_all_recursive(parent_fd: Option<RawFd>, p: &Path) -> io::Result<()> {
+        let pcstr = cstr(p)?;
+
+        // entry is expected to be a directory, open as such
+        let fd = openat_nofollow_dironly(parent_fd, &pcstr)?;
+
+        // open the directory passing ownership of the fd
+        let (dir, fd) = fdreaddir(fd)?;
+        for child in dir {
+            let child = child?;
+            match is_dir(&child) {
+                Some(true) => {
+                    remove_dir_all_recursive(Some(fd), Path::new(&child.file_name()))?;
+                }
+                Some(false) => {
+                    cvt(unsafe { unlinkat(fd, child.name_cstr().as_ptr(), 0) })?;
+                }
+                None => match cvt(unsafe { unlinkat(fd, child.name_cstr().as_ptr(), 0) }) {
+                    // type unknown - try to unlink
+                    Err(err)
+                        if err.raw_os_error() == Some(libc::EISDIR)
+                            || err.raw_os_error() == Some(libc::EPERM) =>
+                    {
+                        // if the file is a directory unlink fails with EISDIR on Linux and EPERM everyhwere else
+                        remove_dir_all_recursive(Some(fd), Path::new(&child.file_name()))?;
+                    }
+                    result => {
+                        result?;
+                    }
+                },
+            }
+        }
+
+        // unlink the directory after removing its contents
+        cvt(unsafe {
+            unlinkat(parent_fd.unwrap_or(libc::AT_FDCWD), pcstr.as_ptr(), libc::AT_REMOVEDIR)
+        })?;
+        Ok(())
+    }
+
+    pub fn remove_dir_all(p: &Path) -> io::Result<()> {
+        // We cannot just call remove_dir_all_recursive() here because that would not delete a passed
+        // symlink. No need to worry about races, because remove_dir_all_recursive() does not recurse
+        // into symlinks.
+        let attr = lstat(p)?;
+        if attr.file_type().is_symlink() {
+            crate::fs::remove_file(p)
+        } else {
+            remove_dir_all_recursive(None, p)
+        }
+    }
+}

library/std/src/sys/unix/weak.rs

@@ -73,12 +73,14 @@ impl<F> ExternWeak<F> {
 
 pub(crate) macro dlsym {
     (fn $name:ident($($t:ty),*) -> $ret:ty) => (
+         dlsym!(fn $name($($t),*) -> $ret, stringify!($name));
+    ),
+    (fn $name:ident($($t:ty),*) -> $ret:ty, $sym:expr) => (
         static DLSYM: DlsymWeak<unsafe extern "C" fn($($t),*) -> $ret> =
-            DlsymWeak::new(concat!(stringify!($name), '\0'));
+            DlsymWeak::new(concat!($sym, '\0'));
         let $name = &DLSYM;
     )
 }
-
 pub(crate) struct DlsymWeak<F> {
     name: &'static str,
     addr: AtomicUsize,