From 0467e77515b8306d6f2a8911118489aff1adbd2a Mon Sep 17 00:00:00 2001 From: Ririsoft Date: Tue, 29 Sep 2026 09:48:56 +0200 Subject: [PATCH 1/9] ignore vim swap files --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 088ba6b..426ef3e 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,6 @@ Cargo.lock # These are backup files generated by rustfmt **/*.rs.bk + +# Vim swap files +*.swp From 2298302f6fa1771406be3dd7eed8cc82b5abaadc Mon Sep 17 00:00:00 2001 From: Ririsoft Date: Tue, 29 Sep 2026 08:34:54 +0200 Subject: [PATCH 2/9] document the symlink TOCTOU race of WalkDir WalkDir checks an entry type without following symlinks, then opens the directory again by path. A concurrent process able to write to the tree can swap the directory for a symlink in between and make the walk escape the root directory. Document the race, who is affected and possible mitigations in the README and crate documentation. --- CHANGELOG.md | 3 +++ README.md | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 12 +++++++++++ 3 files changed, 71 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index afb49d6..959f76a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed ### Security +- Documented a TOCTOU race: a concurrent process able to modify the walked tree can swap + a directory for a symlink between the type check and the directory read, making the + walk escape the root directory. See the README "Security" section. - Hardened CI/CD supply chain: GitHub Actions pinned to commit SHAs, least-privilege workflow permissions, crates.io trusted publishing (OIDC) from a protected `release` environment, Dependabot security updates, and actionlint/zizmor workflow linting. diff --git a/README.md b/README.md index 9fedfeb..0721488 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,60 @@ block_on(async { }); ``` +# Security: symlinks and concurrent modifications + +`WalkDir` does not follow symbolic links: a symlink is yielded as an entry but the walker +never descends into it. This guarantee only holds **while nobody else modifies the +directory tree during the walk**. + +## The race + +For each entry, the walker first checks whether it is a directory without following +symlinks, then opens it again *by path* to list its content. Opening by path resolves every +component of the path again and follows symlinks. Between the check and the open, a +concurrent process can replace a directory with a symlink: + +```text +root/ +└── uploads/ 1. walker checks `root/uploads`: a real directory, ok + 2. attacker: mv root/uploads root/x && ln -s /etc root/uploads + 3. walker opens `root/uploads` → actually lists /etc +``` + +The walker then yields entries located outside of `root`, while their paths still look +like they are inside it (`root/uploads/passwd`). A check such as +`entry.path().starts_with(root)` does not detect it. The swap can be repeated deeper in +the tree, and replacing any *parent* directory has the same effect. + +This is a classic time-of-check to time-of-use (TOCTOU) race. It is the same class of issue +as [CVE-2022-21658][8] in `std::fs::remove_dir_all`, and it also affects the synchronous +[walkdir][1] crate ([BurntSushi/walkdir#209][9]). + +## Am I affected? + +You are affected only if **both** conditions are true: + +- the walked tree can be modified by someone you do not trust (shared or world-writable + directories such as `/tmp`, upload folders, other users' home directories, container + volumes, ...), **and** +- your process has privileges that this someone does not have (runs as root, as a service + account, with access to secrets, ...), and acts on the entries: deleting, copying, + changing ownership or permissions, serving or indexing them. + +Walking your own files, a build directory, or any tree only writable by the user running +the walk is **not** affected. + +## Mitigations + +- Do not walk directories writable by less privileged users from a privileged process. + Drop privileges to the owner of the tree before walking it when possible. +- Remember that acting on `entry.path()` after the walk is racy as well: the path is + resolved again by every subsequent file system call. Re-validate with + `std::fs::symlink_metadata` right before acting and treat any unexpected change as an + error, keeping in mind that this only narrows the window. +- Robust protection requires walking with directory handles (`openat` on Unix, + relative `NtCreateFile` on Windows) instead of paths. + [1]: https://docs.rs/walkdir [2]: https://docs.rs/async-fs [3]: https://docs.rs/blocking @@ -38,3 +92,5 @@ block_on(async { [5]: https://docs.rs/tokio [6]: https://docs.rs/async-std [7]: https://docs.rs/smol +[8]: https://blog.rust-lang.org/2022/01/20/cve-2022-21658.html +[9]: https://github.com/BurntSushi/walkdir/issues/209 diff --git a/src/lib.rs b/src/lib.rs index 97b4a0f..6e3caf2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -22,6 +22,18 @@ //! //! Symbolic links are walked through but they are not followed. //! +//! # Security +//! +//! The "symlinks are not followed" guarantee only holds while the directory tree is not +//! modified concurrently. Directories are checked, then opened again by path: a +//! concurrent process able to write to the tree can swap a directory for a symlink in +//! between and make the walk escape the root directory (a TOCTOU race, similar to +//! [CVE-2022-21658](https://blog.rust-lang.org/2022/01/20/cve-2022-21658.html)). +//! +//! Do not rely on it when a privileged process walks a tree writable by less privileged +//! users. See the [README](https://github.com/ririsoft/async-walkdir#security-symlinks-and-concurrent-modifications) +//! for details and mitigations. +//! //! # Example //! //! Recursively traverse a directory: From 0565d814035b050d8bd28ddfb6ea14800b0fc1c4 Mon Sep 17 00:00:00 2001 From: Ririsoft Date: Tue, 29 Sep 2026 09:24:57 +0200 Subject: [PATCH 3/9] add a secure walker resistant to symlink races on Unix Add the secure module, behind the opt-in secure cargo feature. Its WalkDir walks with directory file descriptors instead of paths: each directory is opened relative to its already opened parent with O_NOFOLLOW | O_DIRECTORY, and metadata is read with fstatat and AT_SYMLINK_NOFOLLOW. A directory swapped for a symlink during the walk is reported as an error instead of being followed, and swapping a parent directory has no effect. Entries expose their parent directory fd and an open() method acting relative to it, so that callers do not have to resolve paths again. FileType and Metadata are crate-owned types since the std ones cannot be built from a directory fd. from_fd() starts a walk from an already opened directory. The existing WalkDir is unchanged. --- Cargo.toml | 12 + src/lib.rs | 7 + src/secure/mod.rs | 564 ++++++++++++++++++++++++++++++++++++++++++++ src/secure/tests.rs | 392 ++++++++++++++++++++++++++++++ src/secure/unix.rs | 304 ++++++++++++++++++++++++ 5 files changed, 1279 insertions(+) create mode 100644 src/secure/mod.rs create mode 100644 src/secure/tests.rs create mode 100644 src/secure/unix.rs diff --git a/Cargo.toml b/Cargo.toml index 43ea76d..f76a323 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,11 +13,23 @@ documentation = "https://docs.rs/async-walkdir/" exclude = [ "/.github/*" ] readme = "README.md" +[features] +# Walker resistant to symlink races, see the `secure` module (Unix only). +secure = ["dep:blocking", "dep:rustix"] + [dependencies] futures-lite = "2.6" async-fs = "2.2" thiserror = "2.0" +blocking = { version = "1.6", optional = true } + +[target.'cfg(unix)'.dependencies] +rustix = { version = "1.1", features = ["fs"], optional = true } [dev-dependencies] tempfile = "3.27" tokio = { version = "1.53", features = ["rt-multi-thread"] } + +[package.metadata.docs.rs] +all-features = true +rustdoc-args = ["--cfg", "docsrs"] diff --git a/src/lib.rs b/src/lib.rs index 6e3caf2..68ebf87 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -34,6 +34,9 @@ //! users. See the [README](https://github.com/ririsoft/async-walkdir#security-symlinks-and-concurrent-modifications) //! for details and mitigations. //! +//! The [`secure`] module, available with the `secure` cargo feature on Unix, provides a +//! walker that is not affected by this race. +//! //! # Example //! //! Recursively traverse a directory: @@ -92,8 +95,12 @@ #![forbid(unsafe_code)] #![deny(missing_docs)] +#![cfg_attr(docsrs, feature(doc_cfg))] mod error; +#[cfg(all(feature = "secure", unix))] +#[cfg_attr(docsrs, doc(cfg(all(feature = "secure", unix))))] +pub mod secure; use std::future::Future; use std::path::{Path, PathBuf}; diff --git a/src/secure/mod.rs b/src/secure/mod.rs new file mode 100644 index 0000000..eca9563 --- /dev/null +++ b/src/secure/mod.rs @@ -0,0 +1,564 @@ +//! A directory walker that never follows symbolic links, even when the +//! tree is modified concurrently. +//! +//! [`crate::WalkDir`] checks each entry, then opens directories again by +//! path. A concurrent process can swap a directory for a symbolic link +//! between those two steps and make the walk escape the root directory +//! (see the crate level [security notes](crate#security)). +//! +//! The [`WalkDir`] of this module walks with directory *handles* instead of +//! paths: +//! +//! - the root directory is opened once, then every child directory is opened +//! *relative to its already opened parent* with symlink following disabled +//! (`openat(2)` with `O_NOFOLLOW` on Unix); +//! - metadata is read relative to the parent directory handle, without +//! following symbolic links; +//! - a directory swapped for a symbolic link between the listing and the +//! opening is detected: the walker yields an error for that entry, does not +//! descend into it, and continues with the next entries; +//! - swapping or moving a *parent* directory has no effect: the walker keeps +//! reading the directories it has opened, wherever they are moved. +//! +//! Each [`DirEntry`] keeps its parent directory handle alive and exposes it, +//! so that you can act on the entry without resolving its path again, which +//! would reintroduce the race in your own code. [`DirEntry::open`] opens a +//! file that way. +//! +//! # Limitations +//! +//! - The root path given to [`WalkDir::new`] is resolved once, following +//! symbolic links: it must be trusted. Use [`WalkDir::from_fd`] to start +//! from a directory handle you already hold. +//! - [`DirEntry::path`] is informational: it is the path the entry had when +//! it was listed. Acting on it resolves it again and is subject to races. +//! - Each directory currently being walked keeps two file descriptors open, +//! and each yielded [`DirEntry`] keeps its parent directory open until it +//! is dropped. +//! +//! # Example +//! +//! ``` +//! use async_walkdir::secure::WalkDir; +//! use futures_lite::future::block_on; +//! use futures_lite::io::AsyncReadExt; +//! use futures_lite::stream::StreamExt; +//! +//! block_on(async { +//! let mut entries = WalkDir::new("my_directory"); +//! while let Some(entry) = entries.next().await { +//! let entry = match entry { +//! Ok(entry) => entry, +//! Err(e) => { +//! eprintln!("error: {}", e); +//! continue; +//! } +//! }; +//! if entry.file_type().await.map(|ft| ft.is_file()).unwrap_or(false) { +//! // Opened relative to its parent directory, never following symlinks. +//! let mut content = String::new(); +//! entry.open().await?.read_to_string(&mut content).await?; +//! } +//! } +//! std::io::Result::Ok(()) +//! }); +//! ``` + +use std::collections::VecDeque; +use std::ffi::OsString; +use std::fmt; +use std::future::Future; +use std::io; +use std::path::{Path, PathBuf}; +use std::pin::Pin; +use std::sync::Arc; +use std::task::{Context, Poll}; +use std::time::SystemTime; + +use blocking::unblock; +use futures_lite::future::Boxed as BoxedFut; +use futures_lite::stream::{self, Stream, StreamExt}; + +use crate::error::InnerError; +use crate::{Filtering, Result}; + +#[cfg(unix)] +use std::os::fd::{AsFd, BorrowedFd, OwnedFd}; + +#[cfg(unix)] +#[path = "unix.rs"] +mod sys; + +#[cfg(test)] +mod tests; + +/// Number of entries read per round trip to the blocking thread pool. +const BATCH_SIZE: usize = 32; + +type BoxStream = futures_lite::stream::Boxed>; + +/// A `Stream` of [`DirEntry`] generated from recursively traversing a +/// directory without ever following symbolic links. +/// +/// This is the race-free counterpart of [`crate::WalkDir`], with the same +/// behavior otherwise: entries are returned without a specific ordering, the +/// root directory itself is not returned, and IO errors are yielded without +/// stopping the walk. +pub struct WalkDir { + root: Root, + entries: BoxStream, +} + +#[derive(Clone)] +enum Root { + Path(PathBuf), + Handle(Arc), +} + +impl WalkDir { + /// Returns a new `WalkDir` starting at `root`. + /// + /// `root` is resolved once when the walk starts, following symbolic + /// links: it must be trusted. + pub fn new(root: impl AsRef) -> Self { + Self::with_root(Root::Path(root.as_ref().to_owned())) + } + + /// Returns a new `WalkDir` starting at the already opened directory `dir`. + /// + /// No path is resolved at all. The paths of the yielded entries are + /// relative to `dir`, for instance `sub/file.txt`. + #[cfg(unix)] + pub fn from_fd(dir: OwnedFd) -> Self { + Self::with_root(Root::Handle(Arc::new(dir))) + } + + fn with_root(root: Root) -> Self { + Self { + entries: walk_dir( + root.clone(), + None:: BoxedFut + Send>>, + ), + root, + } + } + + /// Filter entries. + pub fn filter(self, f: F) -> Self + where + F: FnMut(DirEntry) -> Fut + Send + 'static, + Fut: Future + Send, + { + Self { + entries: walk_dir(self.root.clone(), Some(f)), + root: self.root, + } + } +} + +impl Stream for WalkDir { + type Item = Result; + + fn poll_next(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + let entries = Pin::new(&mut self.entries); + entries.poll_next(cx) + } +} + +/// An entry yielded by [`WalkDir`]. +/// +/// The entry holds its parent directory handle, which stays open as long as +/// the entry (or one of its clones) is alive. +#[derive(Clone)] +pub struct DirEntry { + parent: Arc, + name: OsString, + path: PathBuf, + // File type found while listing the parent directory, when available. + file_type: Option, + // Metadata found while listing the parent directory, when available. + metadata: Option, +} + +impl fmt::Debug for DirEntry { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_tuple("DirEntry").field(&self.path).finish() + } +} + +impl DirEntry { + /// Returns the path the entry had when its parent directory was listed. + /// + /// This path is informational. Opening or modifying the entry through it + /// resolves it again and is subject to races: use [`DirEntry::open`] or + /// the parent directory handle instead. + pub fn path(&self) -> PathBuf { + self.path.clone() + } + + /// Returns the file name of the entry, relative to its parent directory. + pub fn file_name(&self) -> OsString { + self.name.clone() + } + + /// Returns the file type of the entry, without following symbolic links. + /// + /// The file type is usually known from the directory listing and does not + /// require any system call. + pub async fn file_type(&self) -> io::Result { + match self.file_type { + Some(file_type) => Ok(file_type), + None => Ok(self.metadata().await?.file_type()), + } + } + + /// Returns the metadata of the entry, without following symbolic links: + /// the metadata of a symbolic link describes the link itself. + /// + /// The metadata is read relative to the parent directory handle. It is a + /// snapshot: to act on a file consistently with its metadata, use + /// [`DirEntry::open`] and read the metadata of the opened file. On some + /// platforms the metadata is the one found while listing the parent + /// directory. + pub async fn metadata(&self) -> io::Result { + if let Some(metadata) = &self.metadata { + return Ok(metadata.clone()); + } + let (parent, name) = (self.parent.clone(), self.name.clone()); + unblock(move || sys::metadata(&parent, &name)) + .await + .map(Metadata) + } + + /// Opens the entry for reading, relative to its parent directory handle + /// and without following symbolic links. + /// + /// Opening a symbolic link fails. + pub async fn open(&self) -> io::Result { + let (parent, name) = (self.parent.clone(), self.name.clone()); + unblock(move || sys::open_file(&parent, &name)) + .await + .map(async_fs::File::from) + } + + /// Returns the handle of the parent directory of this entry. + /// + /// Combined with [`DirEntry::file_name`], it allows acting on the entry + /// with `*at` system calls (`openat`, `unlinkat`, `fstatat`, ...) without + /// resolving its path again. + #[cfg(unix)] + pub fn parent_fd(&self) -> BorrowedFd<'_> { + self.parent.as_fd() + } +} + +/// The type of a [`DirEntry`], as returned by [`DirEntry::file_type`] and +/// [`Metadata::file_type`]. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub struct FileType(sys::FileType); + +impl FileType { + /// Returns `true` if this is a directory. + pub fn is_dir(&self) -> bool { + self.0.is_dir() + } + + /// Returns `true` if this is a regular file. + pub fn is_file(&self) -> bool { + self.0.is_file() + } + + /// Returns `true` if this is a symbolic link. + pub fn is_symlink(&self) -> bool { + self.0.is_symlink() + } +} + +/// Metadata of a [`DirEntry`], as returned by [`DirEntry::metadata`]. +/// +/// Symbolic links are not followed: the metadata of a symbolic link +/// describes the link itself. +#[derive(Clone, Debug)] +pub struct Metadata(sys::Metadata); + +impl Metadata { + /// Returns the file type. + pub fn file_type(&self) -> FileType { + FileType(self.0.file_type()) + } + + /// Returns `true` if this is the metadata of a directory. + pub fn is_dir(&self) -> bool { + self.file_type().is_dir() + } + + /// Returns `true` if this is the metadata of a regular file. + pub fn is_file(&self) -> bool { + self.file_type().is_file() + } + + /// Returns `true` if this is the metadata of a symbolic link. + pub fn is_symlink(&self) -> bool { + self.file_type().is_symlink() + } + + /// Returns the size in bytes. + #[allow(clippy::len_without_is_empty)] + pub fn len(&self) -> u64 { + self.0.len() + } + + /// Returns `true` if the entry is read-only, with the same semantics as + /// [`std::fs::Permissions::readonly`]. + pub fn readonly(&self) -> bool { + self.0.readonly() + } + + /// Returns the last modification time. + pub fn modified(&self) -> io::Result { + self.0.modified() + } + + /// Returns the last access time. + pub fn accessed(&self) -> io::Result { + self.0.accessed() + } + + /// Returns the creation time. + /// + /// Fails with [`io::ErrorKind::Unsupported`] on platforms or file + /// systems that do not record it. + pub fn created(&self) -> io::Result { + self.0.created() + } +} + +mod sealed { + pub trait Sealed {} + impl Sealed for super::FileType {} + impl Sealed for super::Metadata {} +} + +/// Unix specific extensions to [`FileType`]. +/// +/// This trait is sealed and cannot be implemented outside of this crate. +#[cfg(unix)] +pub trait FileTypeExt: sealed::Sealed { + /// Returns `true` if this is a block device. + fn is_block_device(&self) -> bool; + /// Returns `true` if this is a character device. + fn is_char_device(&self) -> bool; + /// Returns `true` if this is a FIFO (named pipe). + fn is_fifo(&self) -> bool; + /// Returns `true` if this is a socket. + fn is_socket(&self) -> bool; +} + +#[cfg(unix)] +impl FileTypeExt for FileType { + fn is_block_device(&self) -> bool { + self.0.is_block_device() + } + fn is_char_device(&self) -> bool { + self.0.is_char_device() + } + fn is_fifo(&self) -> bool { + self.0.is_fifo() + } + fn is_socket(&self) -> bool { + self.0.is_socket() + } +} + +/// Unix specific extensions to [`Metadata`]. +/// +/// This trait is sealed and cannot be implemented outside of this crate. +#[cfg(unix)] +pub trait MetadataExt: sealed::Sealed { + /// Returns the ID of the device containing the file. + fn dev(&self) -> u64; + /// Returns the inode number. + fn ino(&self) -> u64; + /// Returns the file type and mode bits (`st_mode`). + fn mode(&self) -> u32; + /// Returns the number of hard links. + fn nlink(&self) -> u64; + /// Returns the user ID of the owner. + fn uid(&self) -> u32; + /// Returns the group ID of the owner. + fn gid(&self) -> u32; +} + +#[cfg(unix)] +impl MetadataExt for Metadata { + fn dev(&self) -> u64 { + self.0.dev + } + fn ino(&self) -> u64 { + self.0.ino + } + fn mode(&self) -> u32 { + self.0.mode + } + fn nlink(&self) -> u64 { + self.0.nlink + } + fn uid(&self) -> u32 { + self.0.uid + } + fn gid(&self) -> u32 { + self.0.gid + } +} + +fn walk_dir(root: Root, filter: Option) -> BoxStream +where + F: FnMut(DirEntry) -> Fut + Send + 'static, + Fut: Future + Send, +{ + stream::unfold(State::Start((root, filter)), move |state| async move { + match state { + State::Start((root, filter)) => { + let (path, opened) = match root { + Root::Path(path) => { + let p = path.clone(); + let opened = unblock(move || { + let dir = sys::open_root(&p)?; + let lister = sys::Lister::new(&dir)?; + Ok((Arc::new(dir), lister)) + }) + .await; + (path, opened) + } + Root::Handle(dir) => { + let opened = unblock(move || { + let lister = sys::Lister::new(&dir)?; + Ok((dir, lister)) + }) + .await; + (PathBuf::new(), opened) + } + }; + match opened { + Err(source) => Some((Err(InnerError::Io { path, source }.into()), State::Done)), + Ok((dir, lister)) => walk(vec![Frame::new(path, dir, lister)], filter).await, + } + } + State::Walk((frames, filter)) => walk(frames, filter).await, + State::Done => None, + } + }) + .boxed() +} + +enum State { + Start((Root, Option)), + Walk((Vec, Option)), + Done, +} + +type UnfoldState = (Result, State); + +/// A directory being walked. +struct Frame { + path: PathBuf, + dir: Arc, + // `None` once the directory has been fully listed. + lister: Option, + // Entries listed but not processed yet. + pending: VecDeque>, +} + +impl Frame { + fn new(path: PathBuf, dir: Arc, lister: sys::Lister) -> Self { + Self { + path, + dir, + lister: Some(lister), + pending: VecDeque::new(), + } + } +} + +// Iterative on purpose: filtered-out entries must not grow the stack (see issue #13). +async fn walk(mut frames: Vec, mut filter: Option) -> Option> +where + F: FnMut(DirEntry) -> Fut + Send + 'static, + Fut: Future + Send, +{ + loop { + let frame = frames.last_mut()?; + let raw = match frame.pending.pop_front() { + Some(raw) => raw, + None => { + let Some(mut lister) = frame.lister.take() else { + frames.pop(); + continue; + }; + let (lister, batch) = unblock(move || { + let batch = lister.next_batch(BATCH_SIZE); + (lister, batch) + }) + .await; + // A short batch means the listing is over (end of directory or + // error): dropping the lister releases its file descriptor early. + if batch.len() == BATCH_SIZE { + frame.lister = Some(lister); + } + frame.pending = batch.into(); + continue; + } + }; + let raw = match raw { + Ok(raw) => raw, + Err(source) => { + let path = frame.path.clone(); + return io_error(path, source, frames, filter); + } + }; + let entry = DirEntry { + parent: frame.dir.clone(), + path: frame.path.join(&raw.name), + name: raw.name, + file_type: raw.file_type.map(FileType), + metadata: raw.metadata.map(Metadata), + }; + let ft = match entry.file_type().await { + Ok(ft) => ft, + Err(source) => return io_error(entry.path, source, frames, filter), + }; + let filtering = match filter.as_mut() { + Some(filter) => filter(entry.clone()).await, + None => Filtering::Continue, + }; + if ft.is_dir() && filtering != Filtering::IgnoreDir { + // The file type above comes from the listing and may be stale by now: + // `sys::open_dir` is the actual check, it refuses anything that is not a + // directory at the time of the open, symbolic links included. + let (parent, name) = (entry.parent.clone(), entry.name.clone()); + let opened = unblock(move || { + let dir = sys::open_dir(&parent, &name)?; + let lister = sys::Lister::new(&dir)?; + Ok((Arc::new(dir), lister)) + }) + .await; + match opened { + Ok((dir, lister)) => frames.push(Frame::new(entry.path.clone(), dir, lister)), + Err(source) => return io_error(entry.path, source, frames, filter), + } + } + if filtering == Filtering::Continue { + return Some((Ok(entry), State::Walk((frames, filter)))); + } + } +} + +fn io_error( + path: PathBuf, + source: io::Error, + frames: Vec, + filter: Option, +) -> Option> { + let err = InnerError::Io { path, source }.into(); + Some((Err(err), State::Walk((frames, filter)))) +} diff --git a/src/secure/tests.rs b/src/secure/tests.rs new file mode 100644 index 0000000..4033488 --- /dev/null +++ b/src/secure/tests.rs @@ -0,0 +1,392 @@ +use std::io::{ErrorKind, Result}; +use std::path::{Path, PathBuf}; + +use futures_lite::future::block_on; +use futures_lite::io::AsyncReadExt; +use futures_lite::stream::StreamExt; + +use super::{DirEntry, WalkDir}; +use crate::Filtering; + +/// Walks `wd` to completion, returning the sorted paths of the yielded +/// entries and the paths of the yielded errors. +async fn collect(mut wd: WalkDir) -> (Vec, Vec) { + let (mut entries, mut errors) = (Vec::new(), Vec::new()); + while let Some(entry) = wd.next().await { + match entry { + Ok(entry) => entries.push(entry.path()), + Err(e) => errors.push(e.path().unwrap().to_owned()), + } + } + entries.sort(); + (entries, errors) +} + +/// Creates `root/f1.txt`, `root/d1/f2.txt` and `root/d1/d2/f3.txt`. +async fn make_tree(root: &Path) -> Result<()> { + async_fs::create_dir_all(root.join("d1").join("d2")).await?; + async_fs::write(root.join("f1.txt"), "f1").await?; + async_fs::write(root.join("d1").join("f2.txt"), "f2").await?; + async_fs::write(root.join("d1").join("d2").join("f3.txt"), "f3").await?; + Ok(()) +} + +async fn find(root: &Path, path: &Path) -> DirEntry { + let mut wd = WalkDir::new(root); + while let Some(entry) = wd.next().await { + let entry = entry.unwrap(); + if entry.path() == path { + return entry; + } + } + panic!("{} not found", path.display()); +} + +#[test] +fn walk_dir_empty() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + let mut wd = WalkDir::new(root.path()); + assert!(wd.next().await.is_none()); + Ok(()) + }) +} + +#[test] +fn walk_dir_not_exist() { + block_on(async { + let mut wd = WalkDir::new("foobar"); + match wd.next().await.unwrap() { + Err(e) => { + assert_eq!(e.path().unwrap(), Path::new("foobar")); + assert_eq!(e.io().unwrap().kind(), ErrorKind::NotFound); + } + _ => panic!("want IO error"), + } + assert!(wd.next().await.is_none()); + }) +} + +#[test] +fn walk_dir_files() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + let r = root.path(); + make_tree(r).await?; + let (got, errors) = collect(WalkDir::new(r)).await; + let want = vec![ + r.join("d1"), + r.join("d1").join("d2"), + r.join("d1").join("d2").join("f3.txt"), + r.join("d1").join("f2.txt"), + r.join("f1.txt"), + ]; + assert_eq!(got, want); + assert!(errors.is_empty()); + Ok(()) + }) +} + +#[test] +fn walk_dir_many_entries() -> Result<()> { + block_on(async { + // More entries than a listing batch, to exercise batch boundaries. + let root = tempfile::tempdir()?; + let count = super::BATCH_SIZE * 3 + 1; + for i in 0..count { + async_fs::write(root.path().join(i.to_string()), []).await?; + } + let (got, errors) = collect(WalkDir::new(root.path())).await; + assert_eq!(got.len(), count); + assert!(errors.is_empty()); + Ok(()) + }) +} + +#[test] +fn filter_dirs() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + let r = root.path(); + make_tree(r).await?; + let wd = WalkDir::new(r).filter(|entry| async move { + match entry.file_type().await { + Ok(ft) if ft.is_dir() => Filtering::Ignore, + _ => Filtering::Continue, + } + }); + let (got, _) = collect(wd).await; + let want = vec![ + r.join("d1").join("d2").join("f3.txt"), + r.join("d1").join("f2.txt"), + r.join("f1.txt"), + ]; + assert_eq!(got, want); + Ok(()) + }) +} + +#[test] +fn filter_dirs_no_traverse() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + let r = root.path(); + make_tree(r).await?; + let d2 = r.join("d1").join("d2"); + let wd = WalkDir::new(r).filter(move |entry| { + let d2 = d2.clone(); + async move { + if entry.path() == d2 { + Filtering::IgnoreDir + } else { + Filtering::Continue + } + } + }); + let (got, _) = collect(wd).await; + let want = vec![r.join("d1"), r.join("d1").join("f2.txt"), r.join("f1.txt")]; + assert_eq!(got, want); + Ok(()) + }) +} + +#[test] +fn metadata_and_open() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + make_tree(root.path()).await?; + let entry = find(root.path(), &root.path().join("f1.txt")).await; + assert_eq!(entry.file_name(), "f1.txt"); + let ft = entry.file_type().await?; + assert!(ft.is_file() && !ft.is_dir() && !ft.is_symlink()); + let md = entry.metadata().await?; + assert!(md.is_file()); + assert_eq!(md.len(), 2); + assert!(!md.readonly()); + let modified = md.modified()?; + let elapsed = modified.elapsed().unwrap_or_default(); + assert!(elapsed.as_secs() < 3600, "unexpected mtime {modified:?}"); + let mut content = String::new(); + entry.open().await?.read_to_string(&mut content).await?; + assert_eq!(content, "f1"); + Ok(()) + }) +} + +#[test] +fn entry_outlives_walker() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + make_tree(root.path()).await?; + // `find` drops the walker: the entry keeps its parent directory open. + let entry = find(root.path(), &root.path().join("d1").join("f2.txt")).await; + let mut content = String::new(); + entry.open().await?.read_to_string(&mut content).await?; + assert_eq!(content, "f2"); + Ok(()) + }) +} + +#[cfg(unix)] +mod unix { + use std::io::Result; + use std::os::unix::fs::{symlink, PermissionsExt}; + + use futures_lite::future::block_on; + use futures_lite::io::AsyncReadExt; + use futures_lite::stream::StreamExt; + use rustix::fs::{Mode, OFlags}; + + use super::{collect, find, make_tree}; + use crate::secure::{FileTypeExt, MetadataExt, WalkDir}; + use crate::Filtering; + + #[test] + fn symlink_is_not_followed() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + let outside = tempfile::tempdir()?; + async_fs::write(outside.path().join("secret"), "secret").await?; + let link = root.path().join("link"); + symlink(outside.path(), &link)?; + + let (got, errors) = collect(WalkDir::new(root.path())).await; + assert_eq!(got, vec![link.clone()]); + assert!(errors.is_empty()); + + let entry = find(root.path(), &link).await; + assert!(entry.file_type().await?.is_symlink()); + // The metadata describes the link, not its target. + assert!(entry.metadata().await?.is_symlink()); + assert!(entry.open().await.is_err()); + Ok(()) + }) + } + + /// A directory swapped for a symlink after being listed and before + /// being opened: the walker must report it and not descend into it. + #[test] + fn swapped_dir_is_not_followed() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + let outside = tempfile::tempdir()?; + async_fs::write(outside.path().join("secret"), "secret").await?; + let d1 = root.path().join("d1"); + async_fs::create_dir(&d1).await?; + + // The filter runs between the listing of `root` and the opening of `d1`. + let target = outside.path().to_owned(); + let wd = WalkDir::new(root.path()).filter(move |entry| { + let target = target.clone(); + async move { + std::fs::remove_dir(entry.path()).unwrap(); + symlink(target, entry.path()).unwrap(); + Filtering::Continue + } + }); + let (got, errors) = collect(wd).await; + assert!(got.is_empty(), "walk escaped the root: {got:?}"); + assert_eq!(errors, vec![d1]); + Ok(()) + }) + } + + /// A parent directory swapped for a symlink while being walked: the + /// walker keeps reading the directory it has opened. + #[test] + fn swapped_parent_is_not_followed() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + let r = root.path().to_owned(); + let outside = tempfile::tempdir()?; + async_fs::create_dir_all(outside.path().join("d2")).await?; + async_fs::write(outside.path().join("d2").join("secret"), "s").await?; + make_tree(&r).await?; + + // When reaching `d1/d2`, `d1` is already open: move it away and + // replace it with a symlink before `d2` gets opened. + let d2 = r.join("d1").join("d2"); + let (root_path, target) = (r.clone(), outside.path().to_owned()); + let wd = WalkDir::new(&r).filter(move |entry| { + let (d2, root_path, target) = (d2.clone(), root_path.clone(), target.clone()); + async move { + if entry.path() == d2 { + std::fs::rename(root_path.join("d1"), root_path.join("moved")).unwrap(); + symlink(target, root_path.join("d1")).unwrap(); + } + Filtering::Continue + } + }); + let (got, errors) = collect(wd).await; + assert!(errors.is_empty()); + // `d2` is read from the original, moved, `d1`: the paths are the + // ones from the time of the listing. + assert!(got.contains(&r.join("d1").join("d2").join("f3.txt"))); + assert!(!got.iter().any(|p| p.ends_with("secret")), "{got:?}"); + Ok(()) + }) + } + + #[test] + fn ignore_dir_does_not_read_it() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + let d1 = root.path().join("d1"); + async_fs::create_dir(&d1).await?; + std::fs::set_permissions(&d1, std::fs::Permissions::from_mode(0o222))?; + let wd = WalkDir::new(root.path()).filter(|_| async { Filtering::IgnoreDir }); + let (got, errors) = collect(wd).await; + assert!(got.is_empty() && errors.is_empty()); + Ok(()) + }) + } + + #[test] + fn unreadable_dir_error_path() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + let d1 = root.path().join("d1"); + async_fs::create_dir(&d1).await?; + std::fs::set_permissions(&d1, std::fs::Permissions::from_mode(0o222))?; + let (got, errors) = collect(WalkDir::new(root.path())).await; + // Like `crate::WalkDir`, an unreadable directory is reported instead of yielded. + assert!(got.is_empty()); + assert_eq!(errors, vec![d1]); + Ok(()) + }) + } + + #[test] + fn open_fifo_does_not_block() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + let fifo = root.path().join("fifo"); + // `mknodat` is not available on every Unix: use the `mkfifo` utility. + let status = std::process::Command::new("mkfifo").arg(&fifo).status()?; + assert!(status.success()); + let entry = find(root.path(), &fifo).await; + assert!(entry.file_type().await?.is_fifo()); + // Without `O_NONBLOCK`, opening a FIFO without writer would hang. + let mut buf = Vec::new(); + entry.open().await?.read_to_end(&mut buf).await?; + assert!(buf.is_empty()); + Ok(()) + }) + } + + #[test] + fn parent_fd_and_metadata_ext() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + make_tree(root.path()).await?; + let path = root.path().join("f1.txt"); + let entry = find(root.path(), &path).await; + let fd = rustix::fs::openat( + entry.parent_fd(), + entry.file_name().as_os_str(), + OFlags::RDONLY | OFlags::CLOEXEC, + Mode::empty(), + )?; + let st = rustix::fs::fstat(&fd)?; + let md = entry.metadata().await?; + assert_eq!(md.ino(), st.st_ino as u64); + let std_md = std::fs::symlink_metadata(&path)?; + assert_eq!(md.mode(), std::os::unix::fs::MetadataExt::mode(&std_md)); + assert_eq!(md.uid(), std::os::unix::fs::MetadataExt::uid(&std_md)); + assert_eq!(md.nlink(), 1); + Ok(()) + }) + } + + #[test] + fn from_fd_yields_relative_paths() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + make_tree(root.path()).await?; + let fd = rustix::fs::open( + root.path(), + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::CLOEXEC, + Mode::empty(), + )?; + let (got, errors) = collect(WalkDir::from_fd(fd)).await; + assert!(errors.is_empty()); + assert!(got.contains(&"f1.txt".into())); + assert!(got.contains(&std::path::Path::new("d1").join("d2").join("f3.txt"))); + assert_eq!(got.len(), 5); + Ok(()) + }) + } + + #[test] + fn walker_is_send() { + fn assert_send(_: T) {} + let wd = WalkDir::new("foo"); + assert_send(async move { + let mut wd = wd; + if let Some(Ok(entry)) = wd.next().await { + let _ = entry.metadata().await; + let _ = entry.open().await; + } + }); + } +} diff --git a/src/secure/unix.rs b/src/secure/unix.rs new file mode 100644 index 0000000..4da4575 --- /dev/null +++ b/src/secure/unix.rs @@ -0,0 +1,304 @@ +//! Unix backend of the secure walker, built on `openat(2)` and friends. +//! +//! Every access is relative to an already opened directory file descriptor +//! and names a single path component read from that directory (`readdir(3)` +//! never returns names containing `/`). Combined with `O_NOFOLLOW` and +//! `AT_SYMLINK_NOFOLLOW`, the kernel resolves exactly one component per call +//! and never follows a symbolic link on our behalf. + +use std::ffi::{OsStr, OsString}; +use std::fs::File; +use std::io; +use std::os::fd::OwnedFd; +use std::os::unix::ffi::OsStrExt; +use std::path::Path; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; + +use rustix::fs::{self as rfs, AtFlags, Mode, OFlags}; + +/// An open directory. +pub(super) type Handle = OwnedFd; + +/// Opens the root directory, following symbolic links: callers must trust it. +pub(super) fn open_root(path: &Path) -> io::Result { + let flags = OFlags::RDONLY | OFlags::DIRECTORY | OFlags::CLOEXEC; + Ok(rfs::open(path, flags, Mode::empty())?) +} + +/// Opens the directory `name` of `parent`, refusing anything else. +/// +/// This is the security check of the walker: the file type seen while +/// listing `parent` is only a hint, which a concurrent process may have +/// invalidated since. +/// - `O_NOFOLLOW` makes the call fail if `name` is a symbolic link (`ELOOP`, +/// `EMLINK` on FreeBSD). It only applies to the last path component, which +/// is the only one here. +/// - `O_DIRECTORY` makes it fail with `ENOTDIR` for anything that is not a +/// directory. The check happens before opening, so a FIFO swapped in +/// cannot block the call. +pub(super) fn open_dir(parent: &Handle, name: &OsStr) -> io::Result { + let flags = OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC; + Ok(rfs::openat(parent, name, flags, Mode::empty())?) +} + +/// Opens the entry `name` of `parent` for reading, refusing symbolic links. +pub(super) fn open_file(parent: &Handle, name: &OsStr) -> io::Result { + // - `O_NONBLOCK`: opening a FIFO for reading blocks until a writer shows + // up, which could hang a thread of the blocking pool forever. It is + // cleared right after the open so that the returned file behaves like + // any `std::fs::File`. + // - `O_NOCTTY`: never let a terminal become the controlling terminal of + // the process. + let flags = + OFlags::RDONLY | OFlags::NOFOLLOW | OFlags::CLOEXEC | OFlags::NONBLOCK | OFlags::NOCTTY; + let fd = rfs::openat(parent, name, flags, Mode::empty())?; + let flags = rfs::fcntl_getfl(&fd)?; + rfs::fcntl_setfl(&fd, flags - OFlags::NONBLOCK)?; + Ok(File::from(fd)) +} + +/// An entry read from a directory listing. +pub(super) struct RawEntry { + pub(super) name: OsString, + pub(super) file_type: Option, + // Unix listings do not provide metadata. + pub(super) metadata: Option, +} + +/// Lists the entries of an open directory. +pub(super) struct Lister(rfs::Dir); + +impl Lister { + pub(super) fn new(dir: &Handle) -> io::Result { + // `Dir::read_from` does not `fdopendir` the given descriptor but a new + // one, obtained by opening "." relative to it. The listing then has + // its own read position, which callers holding the directory handle + // (see `DirEntry::parent_fd`) cannot disturb. + Ok(Self(rfs::Dir::read_from(dir)?)) + } + + /// Reads up to `max` entries. Returns less than `max` entries only once + /// the listing is over, either at the end of the directory or after an + /// error, which is then the last element. + pub(super) fn next_batch(&mut self, max: usize) -> Vec> { + let mut batch = Vec::with_capacity(max); + while batch.len() < max { + match self.0.read() { + None => break, + Some(Err(err)) => { + // `Dir` stops listing after an error. + batch.push(Err(err.into())); + break; + } + Some(Ok(entry)) => { + let name = entry.file_name().to_bytes(); + if name == b"." || name == b".." { + continue; + } + // `DT_UNKNOWN` is returned by file systems that do not store + // the file type in directories: fall back to `fstatat`. + let file_type = match entry.file_type() { + rfs::FileType::Unknown => None, + file_type => Some(FileType(file_type)), + }; + batch.push(Ok(RawEntry { + name: OsStr::from_bytes(name).to_owned(), + file_type, + metadata: None, + })); + } + } + } + batch + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(super) struct FileType(rfs::FileType); + +// `rustix::fs::FileType` does not implement `Hash`: hash its discriminant, +// which is consistent with its derived `PartialEq`. +impl std::hash::Hash for FileType { + fn hash(&self, state: &mut H) { + std::mem::discriminant(&self.0).hash(state); + } +} + +impl FileType { + pub(super) fn is_dir(&self) -> bool { + self.0 == rfs::FileType::Directory + } + pub(super) fn is_file(&self) -> bool { + self.0 == rfs::FileType::RegularFile + } + pub(super) fn is_symlink(&self) -> bool { + self.0 == rfs::FileType::Symlink + } + pub(super) fn is_block_device(&self) -> bool { + self.0 == rfs::FileType::BlockDevice + } + pub(super) fn is_char_device(&self) -> bool { + self.0 == rfs::FileType::CharacterDevice + } + pub(super) fn is_fifo(&self) -> bool { + self.0 == rfs::FileType::Fifo + } + pub(super) fn is_socket(&self) -> bool { + self.0 == rfs::FileType::Socket + } +} + +#[derive(Clone, Debug)] +pub(super) struct Metadata { + file_type: FileType, + pub(super) mode: u32, + pub(super) dev: u64, + pub(super) ino: u64, + pub(super) nlink: u64, + pub(super) uid: u32, + pub(super) gid: u32, + size: u64, + accessed: Timestamp, + modified: Timestamp, + created: Option, +} + +impl Metadata { + pub(super) fn file_type(&self) -> FileType { + self.file_type + } + pub(super) fn len(&self) -> u64 { + self.size + } + pub(super) fn readonly(&self) -> bool { + // Same definition as `std::fs::Permissions::readonly` on Unix. + self.mode & 0o222 == 0 + } + pub(super) fn modified(&self) -> io::Result { + self.modified.to_system_time() + } + pub(super) fn accessed(&self) -> io::Result { + self.accessed.to_system_time() + } + pub(super) fn created(&self) -> io::Result { + match self.created { + Some(created) => created.to_system_time(), + None => Err(io::Error::new( + io::ErrorKind::Unsupported, + "creation time is not available on this platform or file system", + )), + } + } + + // The field types of `struct stat` vary across platforms: the casts are + // needed on some of them and redundant on others. + #[allow(clippy::unnecessary_cast)] + fn from_stat(st: &rfs::Stat) -> Self { + Self { + file_type: FileType(rfs::FileType::from_raw_mode(st.st_mode as _)), + mode: st.st_mode as u32, + dev: st.st_dev as u64, + ino: st.st_ino as u64, + nlink: st.st_nlink as u64, + uid: st.st_uid as u32, + gid: st.st_gid as u32, + size: st.st_size as u64, + accessed: Timestamp::new(st.st_atime as i64, st.st_atime_nsec as i64), + modified: Timestamp::new(st.st_mtime as i64, st.st_mtime_nsec as i64), + created: birth_time(st), + } + } +} + +/// Reads the metadata of the entry `name` of `parent`, without following +/// symbolic links. +pub(super) fn metadata(parent: &Handle, name: &OsStr) -> io::Result { + #[cfg(any(target_os = "linux", target_os = "android"))] + match statx_metadata(parent, name) { + Ok(metadata) => return Ok(metadata), + // `statx` is missing on Linux < 4.11, and some seccomp filters (older + // container runtimes) reject it with `EPERM`: fall back to `fstatat`, + // without the creation time. `std` applies the same fallback. + Err(rustix::io::Errno::NOSYS) | Err(rustix::io::Errno::PERM) => {} + Err(err) => return Err(err.into()), + } + let st = rfs::statat(parent, name, AtFlags::SYMLINK_NOFOLLOW)?; + Ok(Metadata::from_stat(&st)) +} + +/// Linux only records the creation time in `statx`. +#[cfg(any(target_os = "linux", target_os = "android"))] +fn statx_metadata(parent: &Handle, name: &OsStr) -> rustix::io::Result { + use rfs::StatxFlags; + + let stx = rfs::statx( + parent, + name, + AtFlags::SYMLINK_NOFOLLOW, + StatxFlags::BASIC_STATS | StatxFlags::BTIME, + )?; + let ts = |t: rfs::StatxTimestamp| Timestamp::new(t.tv_sec, i64::from(t.tv_nsec)); + // The file system may not support the creation time even if asked for. + let created = StatxFlags::from_bits_retain(stx.stx_mask) + .contains(StatxFlags::BTIME) + .then(|| ts(stx.stx_btime)); + Ok(Metadata { + file_type: FileType(rfs::FileType::from_raw_mode(stx.stx_mode.into())), + mode: stx.stx_mode.into(), + dev: rfs::makedev(stx.stx_dev_major, stx.stx_dev_minor), + ino: stx.stx_ino, + nlink: stx.stx_nlink.into(), + uid: stx.stx_uid, + gid: stx.stx_gid, + size: stx.stx_size, + accessed: ts(stx.stx_atime), + modified: ts(stx.stx_mtime), + created, + }) +} + +#[cfg(any(target_vendor = "apple", target_os = "freebsd"))] +#[allow(clippy::unnecessary_cast)] // See `Metadata::from_stat`. +fn birth_time(st: &rfs::Stat) -> Option { + Some(Timestamp::new( + st.st_birthtime as i64, + st.st_birthtime_nsec as i64, + )) +} + +#[cfg(not(any(target_vendor = "apple", target_os = "freebsd")))] +fn birth_time(_: &rfs::Stat) -> Option { + None +} + +/// A time as seconds and nanoseconds since the Unix epoch. +#[derive(Clone, Copy, Debug)] +struct Timestamp { + secs: i64, + nanos: u32, +} + +impl Timestamp { + fn new(secs: i64, nanos: i64) -> Self { + // Guard against out of range values from exotic file systems rather + // than letting `Duration::new` carry them into the seconds. + let nanos = u32::try_from(nanos) + .ok() + .filter(|n| *n < 1_000_000_000) + .unwrap_or(0); + Self { secs, nanos } + } + + fn to_system_time(self) -> io::Result { + // Checked arithmetic: `SystemTime` cannot represent every `i64` of + // seconds, and the unchecked operators would panic. + let nanos = Duration::from_nanos(self.nanos.into()); + let time = if self.secs >= 0 { + UNIX_EPOCH.checked_add(Duration::from_secs(self.secs.unsigned_abs())) + } else { + UNIX_EPOCH.checked_sub(Duration::from_secs(self.secs.unsigned_abs())) + }; + time.and_then(|t| t.checked_add(nanos)) + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "timestamp out of range")) + } +} From 3de65e7c918c8993b795814ce2de7b9be257de4e Mon Sep 17 00:00:00 2001 From: Ririsoft Date: Tue, 29 Sep 2026 09:39:29 +0200 Subject: [PATCH 4/9] add Windows support to the secure walker Walk directories with NtCreateFile relative to the parent directory handle and FILE_OPEN_REPARSE_POINT, then refuse symbolic links and junctions by checking the opened handle. Listings use GetFileInformationByHandleEx(FileIdBothDirectoryInfo) on a separate file object and are parsed with bounds-checked code. unsafe code is now denied instead of forbidden at the crate root so that the Windows backend alone can call Windows APIs, each unsafe block documenting why it is sound. --- Cargo.toml | 14 +- src/lib.rs | 13 +- src/secure/mod.rs | 68 ++++++- src/secure/tests.rs | 186 ++++++++++++++++- src/secure/windows.rs | 453 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 709 insertions(+), 25 deletions(-) create mode 100644 src/secure/windows.rs diff --git a/Cargo.toml b/Cargo.toml index f76a323..45f64f3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,8 +14,8 @@ exclude = [ "/.github/*" ] readme = "README.md" [features] -# Walker resistant to symlink races, see the `secure` module (Unix only). -secure = ["dep:blocking", "dep:rustix"] +# Walker resistant to symlink races, see the `secure` module (Unix and Windows). +secure = ["dep:blocking", "dep:rustix", "dep:windows-sys"] [dependencies] futures-lite = "2.6" @@ -26,6 +26,16 @@ blocking = { version = "1.6", optional = true } [target.'cfg(unix)'.dependencies] rustix = { version = "1.1", features = ["fs"], optional = true } +[target.'cfg(windows)'.dependencies] +windows-sys = { version = "0.61", optional = true, features = [ + "Wdk_Foundation", + "Wdk_Storage_FileSystem", + "Win32_Foundation", + "Win32_Security", + "Win32_Storage_FileSystem", + "Win32_System_IO", +] } + [dev-dependencies] tempfile = "3.27" tokio = { version = "1.53", features = ["rt-multi-thread"] } diff --git a/src/lib.rs b/src/lib.rs index 68ebf87..6ddba15 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -34,8 +34,8 @@ //! users. See the [README](https://github.com/ririsoft/async-walkdir#security-symlinks-and-concurrent-modifications) //! for details and mitigations. //! -//! The [`secure`] module, available with the `secure` cargo feature on Unix, provides a -//! walker that is not affected by this race. +//! The [`secure`] module, available with the `secure` cargo feature on Unix and Windows, +//! provides a walker that is not affected by this race. //! //! # Example //! @@ -93,13 +93,16 @@ //! }); //! ``` -#![forbid(unsafe_code)] +// `unsafe` is only allowed in the Windows backend of the `secure` module, to +// call Windows APIs not exposed by `std`. `forbid` cannot be relaxed locally. +#![deny(unsafe_code)] +#![warn(clippy::undocumented_unsafe_blocks)] #![deny(missing_docs)] #![cfg_attr(docsrs, feature(doc_cfg))] mod error; -#[cfg(all(feature = "secure", unix))] -#[cfg_attr(docsrs, doc(cfg(all(feature = "secure", unix))))] +#[cfg(all(feature = "secure", any(unix, windows)))] +#[cfg_attr(docsrs, doc(cfg(all(feature = "secure", any(unix, windows)))))] pub mod secure; use std::future::Future; diff --git a/src/secure/mod.rs b/src/secure/mod.rs index eca9563..b830486 100644 --- a/src/secure/mod.rs +++ b/src/secure/mod.rs @@ -11,11 +11,12 @@ //! //! - the root directory is opened once, then every child directory is opened //! *relative to its already opened parent* with symlink following disabled -//! (`openat(2)` with `O_NOFOLLOW` on Unix); +//! (`openat(2)` with `O_NOFOLLOW` on Unix, `NtCreateFile` with a root +//! directory handle and `FILE_OPEN_REPARSE_POINT` on Windows); //! - metadata is read relative to the parent directory handle, without //! following symbolic links; -//! - a directory swapped for a symbolic link between the listing and the -//! opening is detected: the walker yields an error for that entry, does not +//! - a directory swapped for a symbolic link (or a junction on Windows) +//! between the listing and the opening is detected: the walker yields an error for that entry, does not //! descend into it, and continues with the next entries; //! - swapping or moving a *parent* directory has no effect: the walker keeps //! reading the directories it has opened, wherever they are moved. @@ -28,13 +29,16 @@ //! # Limitations //! //! - The root path given to [`WalkDir::new`] is resolved once, following -//! symbolic links: it must be trusted. Use [`WalkDir::from_fd`] to start -//! from a directory handle you already hold. +//! symbolic links: it must be trusted. Use `WalkDir::from_fd` (Unix) or +//! `WalkDir::from_handle` (Windows) to start from a directory handle you +//! already hold. //! - [`DirEntry::path`] is informational: it is the path the entry had when //! it was listed. Acting on it resolves it again and is subject to races. -//! - Each directory currently being walked keeps two file descriptors open, -//! and each yielded [`DirEntry`] keeps its parent directory open until it -//! is dropped. +//! - Each directory currently being walked keeps two handles open, and each +//! yielded [`DirEntry`] keeps its parent directory open until it is +//! dropped. +//! - On Windows, [`DirEntry::metadata`] returns the metadata found while +//! listing the parent directory, like [`std::fs::DirEntry::metadata`]. //! //! # Example //! @@ -84,10 +88,15 @@ use crate::{Filtering, Result}; #[cfg(unix)] use std::os::fd::{AsFd, BorrowedFd, OwnedFd}; +#[cfg(windows)] +use std::os::windows::io::{AsHandle, BorrowedHandle, OwnedHandle}; #[cfg(unix)] #[path = "unix.rs"] mod sys; +#[cfg(windows)] +#[path = "windows.rs"] +mod sys; #[cfg(test)] mod tests; @@ -133,6 +142,17 @@ impl WalkDir { Self::with_root(Root::Handle(Arc::new(dir))) } + /// Returns a new `WalkDir` starting at the already opened directory `dir`. + /// + /// No path is resolved at all. The paths of the yielded entries are + /// relative to `dir`, for instance `sub\file.txt`. The handle must have + /// been opened with the `FILE_LIST_DIRECTORY` access right, for instance + /// with [`std::fs::OpenOptions`] and `FILE_FLAG_BACKUP_SEMANTICS`. + #[cfg(windows)] + pub fn from_handle(dir: OwnedHandle) -> Self { + Self::with_root(Root::Handle(Arc::new(dir))) + } + fn with_root(root: Root) -> Self { Self { entries: walk_dir( @@ -250,6 +270,17 @@ impl DirEntry { pub fn parent_fd(&self) -> BorrowedFd<'_> { self.parent.as_fd() } + + /// Returns the handle of the parent directory of this entry. + /// + /// Combined with [`DirEntry::file_name`], it allows acting on the entry + /// with `NtCreateFile` and a root directory handle, without resolving its + /// path again. The handle is opened with the `FILE_LIST_DIRECTORY`, + /// `FILE_READ_ATTRIBUTES` and `SYNCHRONIZE` access rights. + #[cfg(windows)] + pub fn parent_handle(&self) -> BorrowedHandle<'_> { + self.parent.as_handle() + } } /// The type of a [`DirEntry`], as returned by [`DirEntry::file_type`] and @@ -411,6 +442,27 @@ impl MetadataExt for Metadata { } } +/// Windows specific extensions to [`Metadata`]. +/// +/// This trait is sealed and cannot be implemented outside of this crate. +#[cfg(windows)] +pub trait MetadataExt: sealed::Sealed { + /// Returns the file attributes (`FILE_ATTRIBUTE_*` flags). + fn file_attributes(&self) -> u32; + /// Returns the file index (file ID) identifying the file on its volume. + fn file_index(&self) -> u64; +} + +#[cfg(windows)] +impl MetadataExt for Metadata { + fn file_attributes(&self) -> u32 { + self.0.attributes + } + fn file_index(&self) -> u64 { + self.0.file_index + } +} + fn walk_dir(root: Root, filter: Option) -> BoxStream where F: FnMut(DirEntry) -> Fut + Send + 'static, diff --git a/src/secure/tests.rs b/src/secure/tests.rs index 4033488..5ce9884 100644 --- a/src/secure/tests.rs +++ b/src/secure/tests.rs @@ -187,6 +187,19 @@ fn entry_outlives_walker() -> Result<()> { }) } +#[test] +fn walker_is_send() { + fn assert_send(_: T) {} + let wd = WalkDir::new("foo"); + assert_send(async move { + let mut wd = wd; + if let Some(Ok(entry)) = wd.next().await { + let _ = entry.metadata().await; + let _ = entry.open().await; + } + }); +} + #[cfg(unix)] mod unix { use std::io::Result; @@ -194,7 +207,6 @@ mod unix { use futures_lite::future::block_on; use futures_lite::io::AsyncReadExt; - use futures_lite::stream::StreamExt; use rustix::fs::{Mode, OFlags}; use super::{collect, find, make_tree}; @@ -376,17 +388,171 @@ mod unix { Ok(()) }) } +} + +#[cfg(windows)] +mod windows { + use std::io::{Error, Result}; + use std::path::Path; + use std::process::Command; + + use futures_lite::future::block_on; + + use super::{collect, find, make_tree}; + use crate::secure::{MetadataExt, WalkDir}; + use crate::Filtering; + + /// Creates a directory junction, which unlike symbolic links requires no + /// privilege. + fn junction(link: &Path, target: &Path) -> Result<()> { + let status = Command::new("cmd") + .args(["/C", "mklink", "/J"]) + .arg(link) + .arg(target) + .status()?; + if !status.success() { + return Err(Error::other(format!("mklink /J failed: {status}"))); + } + Ok(()) + } + + #[test] + fn junction_is_not_followed() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + let outside = tempfile::tempdir()?; + async_fs::write(outside.path().join("secret"), "secret").await?; + let link = root.path().join("link"); + junction(&link, outside.path())?; + + let (got, errors) = collect(WalkDir::new(root.path())).await; + assert_eq!(got, vec![link.clone()]); + assert!(errors.is_empty()); + + let entry = find(root.path(), &link).await; + assert!(entry.file_type().await?.is_symlink()); + assert!(entry.metadata().await?.is_symlink()); + assert!(entry.open().await.is_err()); + Ok(()) + }) + } #[test] - fn walker_is_send() { - fn assert_send(_: T) {} - let wd = WalkDir::new("foo"); - assert_send(async move { - let mut wd = wd; - if let Some(Ok(entry)) = wd.next().await { - let _ = entry.metadata().await; - let _ = entry.open().await; + fn symlink_is_not_followed() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + let outside = tempfile::tempdir()?; + async_fs::write(outside.path().join("secret"), "secret").await?; + let link = root.path().join("link"); + match std::os::windows::fs::symlink_dir(outside.path(), &link) { + // ERROR_PRIVILEGE_NOT_HELD: symbolic links require the + // developer mode or an elevated process. + Err(e) if e.raw_os_error() == Some(1314) => return Ok(()), + r => r?, } - }); + let (got, errors) = collect(WalkDir::new(root.path())).await; + assert_eq!(got, vec![link]); + assert!(errors.is_empty()); + Ok(()) + }) + } + + /// A directory swapped for a junction after being listed and before + /// being opened: the walker must report it and not descend into it. + #[test] + fn swapped_dir_is_not_followed() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + let outside = tempfile::tempdir()?; + async_fs::write(outside.path().join("secret"), "secret").await?; + let d1 = root.path().join("d1"); + async_fs::create_dir(&d1).await?; + + // The filter runs between the listing of `root` and the opening of `d1`. + let target = outside.path().to_owned(); + let wd = WalkDir::new(root.path()).filter(move |entry| { + let target = target.clone(); + async move { + std::fs::remove_dir(entry.path()).unwrap(); + junction(&entry.path(), &target).unwrap(); + Filtering::Continue + } + }); + let (got, errors) = collect(wd).await; + assert!(got.is_empty(), "walk escaped the root: {got:?}"); + assert_eq!(errors, vec![d1]); + Ok(()) + }) + } + + #[test] + fn open_dir_is_refused() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + make_tree(root.path()).await?; + let entry = find(root.path(), &root.path().join("d1")).await; + assert!(entry.open().await.is_err()); + Ok(()) + }) + } + + #[test] + fn parent_handle_and_metadata_ext() -> Result<()> { + block_on(async { + // `FILE_ATTRIBUTE_READONLY`. + const READONLY: u32 = 0x1; + + let root = tempfile::tempdir()?; + make_tree(root.path()).await?; + let path = root.path().join("f1.txt"); + let mut perms = std::fs::metadata(&path)?.permissions(); + perms.set_readonly(true); + std::fs::set_permissions(&path, perms)?; + + let entry = find(root.path(), &path).await; + let md = entry.metadata().await?; + assert!(md.readonly()); + assert_ne!(md.file_attributes() & READONLY, 0); + assert_ne!(md.file_index(), 0); + + let parent = std::fs::File::from(entry.parent_handle().try_clone_to_owned()?); + assert!(parent.metadata()?.is_dir()); + Ok(()) + }) + } + + #[test] + fn from_handle_yields_relative_paths() -> Result<()> { + use std::os::windows::fs::OpenOptionsExt; + // `FILE_FLAG_BACKUP_SEMANTICS`, required to open a directory. + const BACKUP_SEMANTICS: u32 = 0x0200_0000; + + block_on(async { + let root = tempfile::tempdir()?; + make_tree(root.path()).await?; + let dir = std::fs::OpenOptions::new() + .read(true) + .custom_flags(BACKUP_SEMANTICS) + .open(root.path())?; + let (got, errors) = collect(WalkDir::from_handle(dir.into())).await; + assert!(errors.is_empty()); + assert!(got.contains(&"f1.txt".into())); + assert!(got.contains(&Path::new("d1").join("d2").join("f3.txt"))); + assert_eq!(got.len(), 5); + Ok(()) + }) + } + + #[test] + fn root_file_is_refused() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + make_tree(root.path()).await?; + let file = root.path().join("f1.txt"); + let (got, errors) = collect(WalkDir::new(&file)).await; + assert!(got.is_empty()); + assert_eq!(errors, vec![file]); + Ok(()) + }) } } diff --git a/src/secure/windows.rs b/src/secure/windows.rs new file mode 100644 index 0000000..4ae88c8 --- /dev/null +++ b/src/secure/windows.rs @@ -0,0 +1,453 @@ +//! Windows backend of the secure walker, built on `NtCreateFile` and +//! `GetFileInformationByHandleEx`. +//! +//! Every access is relative to an already opened directory handle +//! (`OBJECT_ATTRIBUTES::RootDirectory`) and names a single path component +//! read from that directory. `FILE_OPEN_REPARSE_POINT` opens symbolic links +//! and junctions themselves instead of their target, and the opened handle is +//! then checked to refuse them. This is the approach used by +//! `std::fs::remove_dir_all` since the fix of CVE-2022-21658. +//! +//! This is the only module of the crate allowed to use `unsafe`, to call +//! Windows APIs that `std` does not expose. Every `unsafe` block documents +//! why it is sound. Parsing of the directory listing is done with safe, +//! bounds-checked code. + +#![allow(unsafe_code)] + +use std::collections::VecDeque; +use std::ffi::{OsStr, OsString}; +use std::fs::{File, OpenOptions}; +use std::io; +use std::mem::{offset_of, size_of}; +use std::os::windows::ffi::{OsStrExt, OsStringExt}; +use std::os::windows::fs::OpenOptionsExt; +use std::os::windows::io::{AsRawHandle, FromRawHandle, OwnedHandle}; +use std::path::Path; +use std::ptr; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; + +use windows_sys::Wdk::Foundation::OBJECT_ATTRIBUTES; +use windows_sys::Wdk::Storage::FileSystem::{ + NtCreateFile, FILE_DIRECTORY_FILE, FILE_NON_DIRECTORY_FILE, FILE_OPEN, + FILE_OPEN_FOR_BACKUP_INTENT, FILE_OPEN_REPARSE_POINT, FILE_SYNCHRONOUS_IO_NONALERT, + NTCREATEFILE_CREATE_OPTIONS, +}; +use windows_sys::Win32::Foundation::{ + RtlNtStatusToDosError, ERROR_DIRECTORY, ERROR_NO_MORE_FILES, ERROR_STOPPED_ON_SYMLINK, HANDLE, + NTSTATUS, UNICODE_STRING, +}; +use windows_sys::Win32::Storage::FileSystem::{ + FileAttributeTagInfo, FileIdBothDirectoryInfo, GetFileInformationByHandleEx, + FILE_ACCESS_RIGHTS, FILE_ATTRIBUTE_DIRECTORY, FILE_ATTRIBUTE_READONLY, + FILE_ATTRIBUTE_REPARSE_POINT, FILE_ATTRIBUTE_TAG_INFO, FILE_FLAG_BACKUP_SEMANTICS, + FILE_GENERIC_READ, FILE_ID_BOTH_DIR_INFO, FILE_LIST_DIRECTORY, FILE_READ_ATTRIBUTES, + FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE, SYNCHRONIZE, +}; +use windows_sys::Win32::System::IO::IO_STATUS_BLOCK; + +/// An open directory. +pub(super) type Handle = OwnedHandle; + +/// Size of the buffer receiving directory listings. 64 KiB is the maximum +/// supported by SMB network shares. +const LISTING_BUFFER_SIZE: usize = 64 * 1024; + +/// Opens the root directory, following symbolic links: callers must trust it. +pub(super) fn open_root(path: &Path) -> io::Result { + // `FILE_FLAG_BACKUP_SEMANTICS` is required to open a directory. + let file = OpenOptions::new() + .read(true) + .custom_flags(FILE_FLAG_BACKUP_SEMANTICS) + .open(path)?; + if !file.metadata()?.is_dir() { + return Err(io::Error::from_raw_os_error(ERROR_DIRECTORY as i32)); + } + Ok(file.into()) +} + +/// Opens the directory `name` of `parent`, refusing anything else. +/// +/// This is the security check of the walker: the file type seen while +/// listing `parent` is only a hint, which a concurrent process may have +/// invalidated since. `FILE_DIRECTORY_FILE` refuses anything that is not a +/// directory, and `check_not_link` refuses symbolic links and junctions. +pub(super) fn open_dir(parent: &Handle, name: &OsStr) -> io::Result { + // `FILE_OPEN_FOR_BACKUP_INTENT` is what `FILE_FLAG_BACKUP_SEMANTICS` maps + // to, so that directories are opened like `std` and `open_root` do. + let dir = nt_open( + parent, + name, + FILE_LIST_DIRECTORY | FILE_READ_ATTRIBUTES, + FILE_DIRECTORY_FILE | FILE_OPEN_FOR_BACKUP_INTENT, + )?; + check_not_link(&dir)?; + Ok(dir) +} + +/// Opens the entry `name` of `parent` for reading, refusing symbolic links +/// and junctions. +pub(super) fn open_file(parent: &Handle, name: &OsStr) -> io::Result { + let file = nt_open(parent, name, FILE_GENERIC_READ, FILE_NON_DIRECTORY_FILE)?; + check_not_link(&file)?; + Ok(file.into()) +} + +/// Windows listings always provide the metadata (see `Lister`), so this +/// fallback, required by the platform independent code, is never reached. +pub(super) fn metadata(_parent: &Handle, _name: &OsStr) -> io::Result { + Err(io::Error::new( + io::ErrorKind::Unsupported, + "metadata is only available from directory listings on Windows", + )) +} + +/// Opens `name` relative to the directory `parent`, without following +/// symbolic links or junctions. +/// +/// An empty `name` opens `parent` itself again, with a new file object. +fn nt_open( + parent: &Handle, + name: &OsStr, + access: FILE_ACCESS_RIGHTS, + options: NTCREATEFILE_CREATE_OPTIONS, +) -> io::Result { + let wide: Vec = name.encode_wide().collect(); + // `UNICODE_STRING` lengths are in bytes, and limited to `u16`. + let len = wide + .len() + .checked_mul(2) + .and_then(|len| u16::try_from(len).ok()) + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "file name too long"))?; + let object_name = UNICODE_STRING { + Length: len, + MaximumLength: len, + // Never written through: `NtCreateFile` takes the name as input only. + Buffer: wide.as_ptr().cast_mut(), + }; + let attributes = OBJECT_ATTRIBUTES { + Length: size_of::() as u32, + RootDirectory: parent.as_raw_handle(), + ObjectName: &object_name, + // No `OBJ_CASE_INSENSITIVE`: `name` is the exact name returned by the + // listing, and a case insensitive lookup could open another entry in + // case sensitive directories. + Attributes: 0, + SecurityDescriptor: ptr::null(), + SecurityQualityOfService: ptr::null(), + }; + let mut handle: HANDLE = ptr::null_mut(); + let mut io_status = IO_STATUS_BLOCK::default(); + // SAFETY: + // - `handle` and `io_status` are valid for writes for the whole call. + // - `attributes` is fully initialized. It points to `object_name`, which + // points to `wide`: all of them outlive the call, and `Length` is the + // size in bytes of `wide` (with a zero length, `Buffer` is never read). + // - `RootDirectory` is a valid directory handle, borrowed from `parent` + // for the duration of the call. + // - The allocation size and extended attributes are optional: null + // pointers with a zero length. + // - `FILE_SYNCHRONOUS_IO_NONALERT` makes the handle synchronous, as + // expected by `std::fs::File`; it requires the `SYNCHRONIZE` right. + let status: NTSTATUS = unsafe { + NtCreateFile( + &mut handle, + access | SYNCHRONIZE, + &attributes, + &mut io_status, + ptr::null(), + 0, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + FILE_OPEN, + options | FILE_OPEN_REPARSE_POINT | FILE_SYNCHRONOUS_IO_NONALERT, + ptr::null(), + 0, + ) + }; + // Negative `NTSTATUS` values are errors (`NT_SUCCESS` macro). + if status < 0 { + // SAFETY: `RtlNtStatusToDosError` only converts an integer. + let code = unsafe { RtlNtStatusToDosError(status) }; + return Err(io::Error::from_raw_os_error(code as i32)); + } + // SAFETY: `NtCreateFile` succeeded, so `handle` is a valid handle that + // nothing else owns: `OwnedHandle` takes care of closing it. + Ok(unsafe { OwnedHandle::from_raw_handle(handle) }) +} + +/// Fails if `handle` is a symbolic link or a junction. +/// +/// `FILE_OPEN_REPARSE_POINT` does not refuse to open links: it opens the +/// link itself instead of its target. Descending into it would not escape +/// the walked tree, but the swap must be reported, not silently ignored. +fn check_not_link(handle: &Handle) -> io::Result<()> { + let mut info = FILE_ATTRIBUTE_TAG_INFO { + FileAttributes: 0, + ReparseTag: 0, + }; + // SAFETY: `handle` is a valid handle opened with `FILE_READ_ATTRIBUTES` + // (included in `FILE_GENERIC_READ`), and `info` is a properly aligned + // `FILE_ATTRIBUTE_TAG_INFO`, valid for writes of the given size, as + // expected for the `FileAttributeTagInfo` class. + let ok = unsafe { + GetFileInformationByHandleEx( + handle.as_raw_handle(), + FileAttributeTagInfo, + (&mut info as *mut FILE_ATTRIBUTE_TAG_INFO).cast(), + size_of::() as u32, + ) + }; + if ok == 0 { + return Err(io::Error::last_os_error()); + } + if is_link(info.FileAttributes, info.ReparseTag) { + return Err(io::Error::from_raw_os_error( + ERROR_STOPPED_ON_SYMLINK as i32, + )); + } + Ok(()) +} + +/// Returns `true` for symbolic links and junctions, with the same +/// definition as `std`: reparse points whose tag is a "name surrogate", i.e. +/// that redirect to another file system location. Other reparse points +/// (deduplicated or cloud files, ...) are regular files and directories. +fn is_link(attributes: u32, reparse_tag: u32) -> bool { + // `IsReparseTagNameSurrogate` macro of `winnt.h`. + const NAME_SURROGATE: u32 = 0x2000_0000; + attributes & FILE_ATTRIBUTE_REPARSE_POINT != 0 && reparse_tag & NAME_SURROGATE != 0 +} + +/// An entry read from a directory listing. +pub(super) struct RawEntry { + pub(super) name: OsString, + pub(super) file_type: Option, + pub(super) metadata: Option, +} + +/// Lists the entries of an open directory. +pub(super) struct Lister { + handle: Handle, + // `u64` elements: `GetFileInformationByHandleEx` requires an 8 bytes + // aligned buffer for `FILE_ID_BOTH_DIR_INFO` records. + buffer: Vec, + pending: VecDeque>, + done: bool, +} + +impl Lister { + pub(super) fn new(dir: &Handle) -> io::Result { + // The listing position is attached to the file object, which handles + // share when duplicated. Opening the directory again gives the + // listing its own position, which callers holding the directory + // handle (see `DirEntry::parent_handle`) cannot disturb. + let handle = nt_open( + dir, + OsStr::new(""), + FILE_LIST_DIRECTORY | FILE_READ_ATTRIBUTES, + FILE_DIRECTORY_FILE | FILE_OPEN_FOR_BACKUP_INTENT, + )?; + Ok(Self { + handle, + buffer: vec![0; LISTING_BUFFER_SIZE / size_of::()], + pending: VecDeque::new(), + done: false, + }) + } + + /// Reads up to `max` entries. Returns less than `max` entries only once + /// the listing is over, either at the end of the directory or after an + /// error, which is then the last element. + pub(super) fn next_batch(&mut self, max: usize) -> Vec> { + while self.pending.len() < max && !self.done { + self.fill(); + } + let len = self.pending.len().min(max); + self.pending.drain(..len).collect() + } + + /// Reads the next chunk of the listing into `self.pending`. + fn fill(&mut self) { + let size = self.buffer.len() * size_of::(); + // SAFETY: `self.handle` is a valid directory handle opened with + // `FILE_LIST_DIRECTORY`, and `self.buffer` is valid for writes of + // `size` bytes and 8 bytes aligned, as `FileIdBothDirectoryInfo` + // requires. Successive calls continue the listing. + let ok = unsafe { + GetFileInformationByHandleEx( + self.handle.as_raw_handle(), + FileIdBothDirectoryInfo, + self.buffer.as_mut_ptr().cast(), + size as u32, + ) + }; + if ok == 0 { + self.done = true; + let err = io::Error::last_os_error(); + if err.raw_os_error() != Some(ERROR_NO_MORE_FILES as i32) { + self.pending.push_back(Err(err)); + } + return; + } + // SAFETY: the buffer is a live, initialized allocation of `size` + // bytes, and `u8` has no alignment or validity requirements. The + // byte view does not outlive this function and the buffer is not + // modified meanwhile. + let bytes = unsafe { std::slice::from_raw_parts(self.buffer.as_ptr().cast::(), size) }; + if let Err(err) = parse_listing(bytes, &mut self.pending) { + self.done = true; + self.pending.push_back(Err(err)); + } + } +} + +/// Parses a chain of `FILE_ID_BOTH_DIR_INFO` records. +/// +/// Fields are read with bounds-checked accessors instead of casting the +/// buffer to the structure, so that a malformed listing can only produce an +/// error, never an out of bounds read. +fn parse_listing(bytes: &[u8], out: &mut VecDeque>) -> io::Result<()> { + type Info = FILE_ID_BOTH_DIR_INFO; + let malformed = || io::Error::new(io::ErrorKind::InvalidData, "malformed directory listing"); + + let mut offset = 0; + loop { + let record = bytes.get(offset..).ok_or_else(malformed)?; + let field_u32 = |at: usize| read_u32(record, at).ok_or_else(malformed); + let field_i64 = |at: usize| read_i64(record, at).ok_or_else(malformed); + + let name_len = field_u32(offset_of!(Info, FileNameLength))? as usize; + let name_start = offset_of!(Info, FileName); + let name_end = name_start.checked_add(name_len).ok_or_else(malformed)?; + let name = record.get(name_start..name_end).ok_or_else(malformed)?; + let name: Vec = name + .as_chunks::<2>() + .0 + .iter() + .map(|c| u16::from_ne_bytes(*c)) + .collect(); + + if name != [u16::from(b'.')] && name != [u16::from(b'.'), u16::from(b'.')] { + let attributes = field_u32(offset_of!(Info, FileAttributes))?; + // For reparse points, `EaSize` holds the reparse tag instead of + // the extended attributes size ([MS-FSCC] 2.4.17). + let reparse_tag = if attributes & FILE_ATTRIBUTE_REPARSE_POINT != 0 { + field_u32(offset_of!(Info, EaSize))? + } else { + 0 + }; + let file_type = FileType::new(attributes, reparse_tag); + let metadata = Metadata { + file_type, + attributes, + size: field_i64(offset_of!(Info, EndOfFile))? as u64, + created: field_i64(offset_of!(Info, CreationTime))?, + accessed: field_i64(offset_of!(Info, LastAccessTime))?, + modified: field_i64(offset_of!(Info, LastWriteTime))?, + file_index: field_i64(offset_of!(Info, FileId))? as u64, + }; + out.push_back(Ok(RawEntry { + name: OsString::from_wide(&name), + file_type: Some(file_type), + metadata: Some(metadata), + })); + } + + match field_u32(offset_of!(Info, NextEntryOffset))? { + 0 => return Ok(()), + next => offset = offset.checked_add(next as usize).ok_or_else(malformed)?, + } + } +} + +fn read_u32(bytes: &[u8], at: usize) -> Option { + Some(u32::from_ne_bytes(bytes.get(at..at + 4)?.try_into().ok()?)) +} + +fn read_i64(bytes: &[u8], at: usize) -> Option { + Some(i64::from_ne_bytes(bytes.get(at..at + 8)?.try_into().ok()?)) +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub(super) struct FileType { + is_directory: bool, + is_link: bool, +} + +impl FileType { + fn new(attributes: u32, reparse_tag: u32) -> Self { + Self { + is_directory: attributes & FILE_ATTRIBUTE_DIRECTORY != 0, + is_link: is_link(attributes, reparse_tag), + } + } + // Same semantics as `std` on Windows: a link to a directory is a symbolic + // link, neither a directory nor a file. + pub(super) fn is_dir(&self) -> bool { + !self.is_link && self.is_directory + } + pub(super) fn is_file(&self) -> bool { + !self.is_link && !self.is_directory + } + pub(super) fn is_symlink(&self) -> bool { + self.is_link + } +} + +#[derive(Clone, Debug)] +pub(super) struct Metadata { + file_type: FileType, + pub(super) attributes: u32, + pub(super) file_index: u64, + size: u64, + // `FILETIME`s: 100 ns intervals since 1601-01-01 UTC. + created: i64, + accessed: i64, + modified: i64, +} + +impl Metadata { + pub(super) fn file_type(&self) -> FileType { + self.file_type + } + pub(super) fn len(&self) -> u64 { + self.size + } + pub(super) fn readonly(&self) -> bool { + // Same definition as `std::fs::Permissions::readonly` on Windows. + self.attributes & FILE_ATTRIBUTE_READONLY != 0 + } + pub(super) fn modified(&self) -> io::Result { + filetime_to_system_time(self.modified) + } + pub(super) fn accessed(&self) -> io::Result { + filetime_to_system_time(self.accessed) + } + pub(super) fn created(&self) -> io::Result { + filetime_to_system_time(self.created) + } +} + +fn filetime_to_system_time(filetime: i64) -> io::Result { + const INTERVALS_PER_SEC: u64 = 10_000_000; + // Number of 100 ns intervals between 1601-01-01 and 1970-01-01. + const UNIX_EPOCH_AS_FILETIME: i64 = 116_444_736_000_000_000; + + let out_of_range = || io::Error::new(io::ErrorKind::InvalidData, "timestamp out of range"); + let since_unix_epoch = filetime + .checked_sub(UNIX_EPOCH_AS_FILETIME) + .ok_or_else(out_of_range)?; + let abs = since_unix_epoch.unsigned_abs(); + // Split in seconds and nanoseconds: converting all the intervals to + // nanoseconds at once could overflow `u64`. + let duration = Duration::new( + abs / INTERVALS_PER_SEC, + ((abs % INTERVALS_PER_SEC) * 100) as u32, + ); + let time = if since_unix_epoch >= 0 { + UNIX_EPOCH.checked_add(duration) + } else { + UNIX_EPOCH.checked_sub(duration) + }; + time.ok_or_else(out_of_range) +} From 06d7197199ec9ce7eb13b9fcf8a944f7222221a8 Mon Sep 17 00:00:00 2001 From: Ririsoft Date: Tue, 29 Sep 2026 09:43:28 +0200 Subject: [PATCH 5/9] extend the tests of the secure walker Cover the entry accessors, including timestamps before the Unix epoch, so that the secure module meets the coverage threshold of the CI. Check that every entry is yielded exactly once around the listing batch size, and that a root handle which is not a directory yields a single error with an empty path. --- src/secure/tests.rs | 114 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 105 insertions(+), 9 deletions(-) diff --git a/src/secure/tests.rs b/src/secure/tests.rs index 5ce9884..67711f1 100644 --- a/src/secure/tests.rs +++ b/src/secure/tests.rs @@ -88,17 +88,26 @@ fn walk_dir_files() -> Result<()> { } #[test] -fn walk_dir_many_entries() -> Result<()> { +fn walk_dir_batch_boundaries() -> Result<()> { + use std::collections::BTreeSet; + block_on(async { - // More entries than a listing batch, to exercise batch boundaries. - let root = tempfile::tempdir()?; - let count = super::BATCH_SIZE * 3 + 1; - for i in 0..count { - async_fs::write(root.path().join(i.to_string()), []).await?; + // Around the size of a listing batch: every entry must be yielded + // exactly once, whatever the number of batches. + let batch = super::BATCH_SIZE; + for count in [batch - 1, batch, batch + 1, 2 * batch + 1] { + let root = tempfile::tempdir()?; + let want: BTreeSet<_> = (0..count) + .map(|i| root.path().join(i.to_string())) + .collect(); + for path in &want { + async_fs::write(path, []).await?; + } + let (got, errors) = collect(WalkDir::new(root.path())).await; + assert!(errors.is_empty()); + assert_eq!(got.len(), count, "duplicate or missing entries"); + assert_eq!(got.into_iter().collect::>(), want); } - let (got, errors) = collect(WalkDir::new(root.path())).await; - assert_eq!(got.len(), count); - assert!(errors.is_empty()); Ok(()) }) } @@ -187,6 +196,42 @@ fn entry_outlives_walker() -> Result<()> { }) } +#[test] +fn entry_accessors() -> Result<()> { + use std::collections::HashSet; + use std::time::{Duration, UNIX_EPOCH}; + + block_on(async { + let root = tempfile::tempdir()?; + make_tree(root.path()).await?; + // A modification time before the Unix epoch, to exercise the + // conversion of negative timestamps. + let before_epoch = UNIX_EPOCH - Duration::from_secs(86_400); + std::fs::File::options() + .write(true) + .open(root.path().join("f1.txt"))? + .set_modified(before_epoch)?; + + let dir = find(root.path(), &root.path().join("d1")).await; + assert!(format!("{dir:?}").contains("d1")); + let dir_md = dir.metadata().await?; + assert!(dir_md.is_dir() && !dir_md.is_file() && !dir_md.is_symlink()); + dir_md.accessed()?; + // The creation time is not recorded everywhere. + if let Err(e) = dir_md.created() { + assert_eq!(e.kind(), ErrorKind::Unsupported); + } + + let file = find(root.path(), &root.path().join("f1.txt")).await; + let file_md = file.metadata().await?; + assert_eq!(file_md.modified()?, before_epoch); + + let types: HashSet<_> = [dir_md.file_type(), file_md.file_type()].into(); + assert_eq!(types.len(), 2); + Ok(()) + }) +} + #[test] fn walker_is_send() { fn assert_send(_: T) {} @@ -207,6 +252,7 @@ mod unix { use futures_lite::future::block_on; use futures_lite::io::AsyncReadExt; + use futures_lite::stream::StreamExt; use rustix::fs::{Mode, OFlags}; use super::{collect, find, make_tree}; @@ -370,6 +416,39 @@ mod unix { }) } + #[test] + fn file_type_ext() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + make_tree(root.path()).await?; + let entry = find(root.path(), &root.path().join("f1.txt")).await; + let ft = entry.file_type().await?; + assert!(!ft.is_fifo() && !ft.is_socket()); + assert!(!ft.is_block_device() && !ft.is_char_device()); + let md = entry.metadata().await?; + let std_md = std::fs::symlink_metadata(entry.path())?; + assert_eq!(md.dev(), std::os::unix::fs::MetadataExt::dev(&std_md)); + assert_eq!(md.gid(), std::os::unix::fs::MetadataExt::gid(&std_md)); + Ok(()) + }) + } + + #[test] + fn from_fd_not_a_dir() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + make_tree(root.path()).await?; + let file = std::fs::File::open(root.path().join("f1.txt"))?; + let mut wd = WalkDir::from_fd(file.into()); + match wd.next().await { + Some(Err(e)) => assert_eq!(e.path().unwrap(), std::path::Path::new("")), + other => panic!("want an error, got {other:?}"), + } + assert!(wd.next().await.is_none()); + Ok(()) + }) + } + #[test] fn from_fd_yields_relative_paths() -> Result<()> { block_on(async { @@ -397,6 +476,7 @@ mod windows { use std::process::Command; use futures_lite::future::block_on; + use futures_lite::stream::StreamExt; use super::{collect, find, make_tree}; use crate::secure::{MetadataExt, WalkDir}; @@ -521,6 +601,22 @@ mod windows { }) } + #[test] + fn from_handle_not_a_dir() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + make_tree(root.path()).await?; + let file = std::fs::File::open(root.path().join("f1.txt"))?; + let mut wd = WalkDir::from_handle(file.into()); + match wd.next().await { + Some(Err(e)) => assert_eq!(e.path().unwrap(), Path::new("")), + other => panic!("want an error, got {other:?}"), + } + assert!(wd.next().await.is_none()); + Ok(()) + }) + } + #[test] fn from_handle_yields_relative_paths() -> Result<()> { use std::os::windows::fs::OpenOptionsExt; From 178aae6ecbe676f81a04689cb85aaacfb304ab1f Mon Sep 17 00:00:00 2001 From: Ririsoft Date: Tue, 29 Sep 2026 09:43:28 +0200 Subject: [PATCH 6/9] run the CI with the secure feature and on macOS Lint all targets with and without the secure feature, run the tests with the feature, include it in the coverage measurement, and add macOS to the matrix to test the BSD flavor of the Unix backend. --- .github/workflows/rust.yml | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index c74c7d3..e151df3 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -17,7 +17,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-latest, windows-latest] + os: [ubuntu-latest, windows-latest, macos-latest] steps: @@ -42,7 +42,10 @@ jobs: run: cargo fmt --all -- --check - name: Run cargo clippy - run: cargo clippy --no-deps -- -D warnings + run: cargo clippy --no-deps --all-targets -- -D warnings + + - name: Run cargo clippy (secure feature) + run: cargo clippy --no-deps --all-targets --features secure -- -D warnings - name: Run cargo build run: cargo build @@ -50,12 +53,15 @@ jobs: - name: Cargo tests run: cargo test + - name: Cargo tests (secure feature) + run: cargo test --features secure + - name: Setup cargo-llvm-cov run: cargo install cargo-llvm-cov --locked --version 0.9.1 - name: Run code coverage test run: | - cargo llvm-cov test --fail-under-lines 90 + cargo llvm-cov test --features secure --fail-under-lines 90 workflow-lint: runs-on: ubuntu-latest From ac2f88867dc199300f6f0ca010d33dbcc6c44013 Mon Sep 17 00:00:00 2001 From: Ririsoft Date: Tue, 29 Sep 2026 09:44:18 +0200 Subject: [PATCH 7/9] document the secure walker Describe the secure feature in the README with an example, a migration table from WalkDir and its limitations, and add it to the changelog. Mark it as experimental and looking for feedback, and explain that it will move to the std directory handle APIs once they are stable. Warn about retained handles on large trees and about the parent directory handle being outside of the walker guarantees. --- CHANGELOG.md | 10 +++++- Cargo.toml | 2 +- README.md | 90 +++++++++++++++++++++++++++++++++++++++++++++-- src/lib.rs | 3 +- src/secure/mod.rs | 42 +++++++++++++++++++--- 5 files changed, 138 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 959f76a..3ec2123 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added + +- Experimental `secure` Cargo feature (Unix and Windows) providing `async_walkdir::secure::WalkDir`, a + walker based on directory handles that is not affected by the symlink TOCTOU race of + `WalkDir`. Its API may still change until it is declared stable, and its implementation + will move to `std` directory handles once available (rust-lang/rust#120426). Feedback is + welcome. See the README "The `secure` walker" section. + ### Fixed ### Changed ### Deprecated @@ -15,7 +22,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Documented a TOCTOU race: a concurrent process able to modify the walked tree can swap a directory for a symlink between the type check and the directory read, making the - walk escape the root directory. See the README "Security" section. + walk escape the root directory. See the README "Security" section, and the new `secure` + walker which is not affected. - Hardened CI/CD supply chain: GitHub Actions pinned to commit SHAs, least-privilege workflow permissions, crates.io trusted publishing (OIDC) from a protected `release` environment, Dependabot security updates, and actionlint/zizmor workflow linting. diff --git a/Cargo.toml b/Cargo.toml index 45f64f3..f203b13 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,7 +14,7 @@ exclude = [ "/.github/*" ] readme = "README.md" [features] -# Walker resistant to symlink races, see the `secure` module (Unix and Windows). +# Experimental walker resistant to symlink races, see the `secure` module (Unix and Windows). secure = ["dep:blocking", "dep:rustix", "dep:windows-sys"] [dependencies] diff --git a/README.md b/README.md index 0721488..b41f707 100644 --- a/README.md +++ b/README.md @@ -82,8 +82,90 @@ the walk is **not** affected. resolved again by every subsequent file system call. Re-validate with `std::fs::symlink_metadata` right before acting and treat any unexpected change as an error, keeping in mind that this only narrows the window. -- Robust protection requires walking with directory handles (`openat` on Unix, - relative `NtCreateFile` on Windows) instead of paths. +- Use the `secure` walker described below, which is not affected by this race. + +## The `secure` walker + +> **Experimental:** the `secure` walker is new and looking for feedback. Its API may still +> change, including in minor releases, until it is declared stable. Please share your use +> cases, problems and suggestions by [opening an issue][10]. + +The opt-in `secure` Cargo feature provides `async_walkdir::secure::WalkDir`, available on +Unix and Windows. It walks the tree with directory handles instead of paths, the same +technique used by `std` to fix [CVE-2022-21658][8]: + +- every directory is opened *relative to its already opened parent*, by a single name, + without following symlinks (`openat` with `O_NOFOLLOW` on Unix, `NtCreateFile` with a + root directory handle and `FILE_OPEN_REPARSE_POINT` on Windows); +- a directory swapped for a symlink (or a junction) after being listed is detected: the + walker yields an error for it, does not descend into it, and continues the walk; +- replacing a parent directory has no effect, since the walker only uses the handle it + has already opened. + +```toml +[dependencies] +async-walkdir = { version = "2", features = ["secure"] } +``` + +```rust +use async_walkdir::secure::WalkDir; +use futures_lite::future::block_on; +use futures_lite::io::AsyncReadExt; +use futures_lite::stream::StreamExt; + +block_on(async { + let mut entries = WalkDir::new("my_directory"); + while let Some(entry) = entries.next().await { + let entry = entry?; + if entry.file_type().await?.is_file() { + // Opened relative to the parent directory handle, never following symlinks. + let mut content = Vec::new(); + entry.open().await?.read_to_end(&mut content).await?; + } + } + Ok::<_, Box>(()) +}); +``` + +Entries also give access to their parent directory handle (`parent_fd` on Unix, +`parent_handle` on Windows), to act on them with other `*at` system calls instead of their +path. The walker's guarantees do not extend to what you do with these handles: resolving +`..` or a name made of several path components relative to them can leave the walked tree. + +### Migrating from `WalkDir` + +The `secure` walker is a separate type: `async_walkdir::WalkDir` is unchanged. + +| `async_walkdir` | `async_walkdir::secure` | +|-------------------------------|--------------------------------------------------------------| +| `WalkDir::new(root)` | `WalkDir::new(root)`, or `WalkDir::from_fd` / `from_handle` | +| `WalkDir::filter(f)` | `WalkDir::filter(f)`, with the same `Filtering` values | +| `DirEntry::path()` | `DirEntry::path()`, for display only: acting on it is racy | +| `DirEntry::file_name()` | `DirEntry::file_name()` | +| `DirEntry::file_type()` | `DirEntry::file_type()`, returning `secure::FileType` | +| `DirEntry::metadata()` (follows symlinks) | `DirEntry::metadata()`, returning `secure::Metadata`, never following symlinks | +| `async_fs::File::open(entry.path())` | `DirEntry::open()` | + +Limitations: + +- The root directory itself is resolved by path, following symlinks: it must be trusted. + Use `WalkDir::from_fd` (Unix) or `WalkDir::from_handle` (Windows) to start from a handle. +- Paths of yielded entries are the ones seen when their parent was listed. Once the walk + moves on, the tree may change: use `DirEntry::open` or the parent handle, not the path. +- Each directory being walked keeps two handles open, and each yielded entry keeps its + parent directory open until it is dropped. Collecting all the entries of a large tree + therefore keeps one handle per directory open and may exceed the limit of open files of + the process (`ulimit -n` on Unix): process entries as they are yielded and drop them. + +### Evolution with the standard library + +The standard library does not expose directory handle APIs yet, so the `secure` walker +relies on [rustix][11] on Unix and on Windows APIs called through [windows-sys][12], which +requires `unsafe` code. Directory handles are being added to `std` +([rust-lang/rust#120426][13]). The `secure` walker will evolve along with them: its +implementation will move to `std` once they are stable, dropping these dependencies and the +`unsafe` code, and its API may be aligned with the `std` one. No `rustix` or `windows-sys` +type is exposed in its API, so that this move does not break your code. [1]: https://docs.rs/walkdir [2]: https://docs.rs/async-fs @@ -94,3 +176,7 @@ the walk is **not** affected. [7]: https://docs.rs/smol [8]: https://blog.rust-lang.org/2022/01/20/cve-2022-21658.html [9]: https://github.com/BurntSushi/walkdir/issues/209 +[10]: https://github.com/ririsoft/async-walkdir/issues +[11]: https://docs.rs/rustix +[12]: https://docs.rs/windows-sys +[13]: https://github.com/rust-lang/rust/issues/120426 diff --git a/src/lib.rs b/src/lib.rs index 6ddba15..408e39f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -35,7 +35,8 @@ //! for details and mitigations. //! //! The [`secure`] module, available with the `secure` cargo feature on Unix and Windows, -//! provides a walker that is not affected by this race. +//! provides a walker that is not affected by this race. It is experimental and looking for +//! feedback. //! //! # Example //! diff --git a/src/secure/mod.rs b/src/secure/mod.rs index b830486..2877bd6 100644 --- a/src/secure/mod.rs +++ b/src/secure/mod.rs @@ -1,6 +1,12 @@ //! A directory walker that never follows symbolic links, even when the //! tree is modified concurrently. //! +//! **Experimental:** this module is new and looking for feedback. Its API may +//! still change, including in minor releases, until it is declared stable. +//! Please share your use cases, problems and suggestions by +//! [opening an issue](https://github.com/ririsoft/async-walkdir/issues) on +//! GitHub. +//! //! [`crate::WalkDir`] checks each entry, then opens directories again by //! path. A concurrent process can swap a directory for a symbolic link //! between those two steps and make the walk escape the root directory @@ -36,10 +42,27 @@ //! it was listed. Acting on it resolves it again and is subject to races. //! - Each directory currently being walked keeps two handles open, and each //! yielded [`DirEntry`] keeps its parent directory open until it is -//! dropped. +//! dropped. Collecting all the entries of a large tree therefore keeps one +//! handle per directory open and may exceed the limit of open files of the +//! process (`ulimit -n` on Unix): process entries as they are yielded and +//! drop them. //! - On Windows, [`DirEntry::metadata`] returns the metadata found while //! listing the parent directory, like [`std::fs::DirEntry::metadata`]. //! +//! # Evolution with the standard library +//! +//! The standard library does not expose directory handle APIs yet: this +//! module relies on `rustix` on Unix and on Windows APIs called through +//! `windows-sys`, which requires `unsafe` code. Directory handles are being +//! added to `std` ([rust-lang/rust#120426]). This module will evolve along +//! with them: its implementation will move to `std` once they are stable, +//! dropping the platform specific dependencies and the `unsafe` code, and +//! its API may be aligned with the `std` one. No `rustix` or `windows-sys` +//! type is exposed, so that the implementation can change without breaking +//! your code. +//! +//! [rust-lang/rust#120426]: https://github.com/rust-lang/rust/issues/120426 +//! //! # Example //! //! ``` @@ -110,7 +133,7 @@ type BoxStream = futures_lite::stream::Boxed>; /// directory without ever following symbolic links. /// /// This is the race-free counterpart of [`crate::WalkDir`], with the same -/// behavior otherwise: entries are returned without a specific ordering, the +/// traversal and filtering behavior: entries are returned without a specific ordering, the /// root directory itself is not returned, and IO errors are yielded without /// stopping the walk. pub struct WalkDir { @@ -136,7 +159,8 @@ impl WalkDir { /// Returns a new `WalkDir` starting at the already opened directory `dir`. /// /// No path is resolved at all. The paths of the yielded entries are - /// relative to `dir`, for instance `sub/file.txt`. + /// relative to `dir`, for instance `sub/file.txt`, and an error about + /// `dir` itself, for instance if it is not a directory, has an empty path. #[cfg(unix)] pub fn from_fd(dir: OwnedFd) -> Self { Self::with_root(Root::Handle(Arc::new(dir))) @@ -145,7 +169,9 @@ impl WalkDir { /// Returns a new `WalkDir` starting at the already opened directory `dir`. /// /// No path is resolved at all. The paths of the yielded entries are - /// relative to `dir`, for instance `sub\file.txt`. The handle must have + /// relative to `dir`, for instance `sub\file.txt`, and an error about + /// `dir` itself, for instance if it is not a directory, has an empty path. + /// The handle must have /// been opened with the `FILE_LIST_DIRECTORY` access right, for instance /// with [`std::fs::OpenOptions`] and `FILE_FLAG_BACKUP_SEMANTICS`. #[cfg(windows)] @@ -266,6 +292,10 @@ impl DirEntry { /// Combined with [`DirEntry::file_name`], it allows acting on the entry /// with `*at` system calls (`openat`, `unlinkat`, `fstatat`, ...) without /// resolving its path again. + /// + /// The walker's guarantees do not extend to what you do with this + /// handle: resolving `..` or a name made of several path components + /// relative to it can leave the walked tree. #[cfg(unix)] pub fn parent_fd(&self) -> BorrowedFd<'_> { self.parent.as_fd() @@ -277,6 +307,10 @@ impl DirEntry { /// with `NtCreateFile` and a root directory handle, without resolving its /// path again. The handle is opened with the `FILE_LIST_DIRECTORY`, /// `FILE_READ_ATTRIBUTES` and `SYNCHRONIZE` access rights. + /// + /// The walker's guarantees do not extend to what you do with this + /// handle: resolving `..` or a name made of several path components + /// relative to it can leave the walked tree. #[cfg(windows)] pub fn parent_handle(&self) -> BorrowedHandle<'_> { self.parent.as_handle() From 5a0ad9b1fe85b458ecea7899bd985af08b04ec4a Mon Sep 17 00:00:00 2001 From: Ririsoft Date: Tue, 29 Sep 2026 10:07:48 +0200 Subject: [PATCH 8/9] test the known symlink race of WalkDir Swap a directory for a symlink (a junction on Windows) between its type check and its opening, and check that the walk escapes the root. This demonstrates the documented limitation, and the README points to this test and to its secure walker counterpart. --- README.md | 7 +++++++ src/lib.rs | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/README.md b/README.md index b41f707..da7f9b2 100644 --- a/README.md +++ b/README.md @@ -56,6 +56,13 @@ like they are inside it (`root/uploads/passwd`). A check such as `entry.path().starts_with(root)` does not detect it. The swap can be repeated deeper in the tree, and replacing any *parent* directory has the same effect. +The unit test `known_limitation_swapped_dir_is_followed` in [`src/lib.rs`](src/lib.rs) +reproduces this attack deterministically on Unix and Windows. It swaps a directory for a +symlink (a junction on Windows) at the right time and checks that the walk escapes the +root. Run it with `cargo test known_limitation`. The `secure` walker described below +passes the same scenario without escaping: run +`cargo test --features secure swapped_dir_is_not_followed`. + This is a classic time-of-check to time-of-use (TOCTOU) race. It is the same class of issue as [CVE-2022-21658][8] in `std::fs::remove_dir_all`, and it also affects the synchronous [walkdir][1] crate ([BurntSushi/walkdir#209][9]). diff --git a/src/lib.rs b/src/lib.rs index 408e39f..4fd8456 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -453,6 +453,66 @@ mod tests { Ok(()) }) } + + /// Replaces the directory `link` with a link to `target`: a symbolic link + /// on Unix, a junction on Windows (which requires no privilege). + #[cfg(any(unix, windows))] + fn swap_for_link(link: &std::path::Path, target: &std::path::Path) -> Result<()> { + std::fs::remove_dir(link)?; + #[cfg(unix)] + std::os::unix::fs::symlink(target, link)?; + #[cfg(windows)] + { + let status = std::process::Command::new("cmd") + .args(["/C", "mklink", "/J"]) + .arg(link) + .arg(target) + .status()?; + assert!(status.success(), "mklink /J failed: {status}"); + } + Ok(()) + } + + /// Demonstrates the known symlink TOCTOU race of `WalkDir`, documented in + /// the README "Security" section and fixed by the `secure` walker (see its + /// `swapped_dir_is_not_followed` tests). + /// + /// The filter runs after `WalkDir` has checked that `d1` is a directory + /// and before it opens `d1` by path: swapping `d1` for a link there makes + /// the walk escape the root. If `WalkDir` ever gets fixed, this test fails + /// and must be inverted, and the documentation updated. + #[cfg(any(unix, windows))] + #[test] + fn known_limitation_swapped_dir_is_followed() -> Result<()> { + block_on(async { + let root = tempfile::tempdir()?; + let outside = tempfile::tempdir()?; + async_fs::write(outside.path().join("secret"), "secret").await?; + let d1 = root.path().join("d1"); + async_fs::create_dir(&d1).await?; + + let (swapped, target) = (d1.clone(), outside.path().to_owned()); + let mut wd = WalkDir::new(root.path()).filter(move |entry| { + let (swapped, target) = (swapped.clone(), target.clone()); + async move { + if entry.path() == swapped { + swap_for_link(&swapped, &target).unwrap(); + } + Filtering::Continue + } + }); + let mut got = Vec::new(); + while let Some(entry) = wd.next().await { + got.push(entry.unwrap().path()); + } + // The secret file located outside of the root is yielded, with a + // path that looks like it is inside the root. + let escaped = d1.join("secret"); + assert!(escaped.starts_with(root.path())); + assert!(got.contains(&escaped), "walk did not escape: {got:?}"); + Ok(()) + }) + } } #[cfg(all(unix, test))] From c280f22d331f471db4b670082f83009c5944b169 Mon Sep 17 00:00:00 2001 From: Ririsoft Date: Tue, 29 Sep 2026 11:24:50 +0200 Subject: [PATCH 9/9] declare Rust 1.85 as the minimum supported version 1.85 is already required by the dependencies. Avoid slice::as_chunks in the Windows backend, which would require 1.88. Following the Cargo book recommendation, check the MSRV in the CI with cargo check on a single runner, type-checking the Windows and macOS backends through their targets, and document the MSRV policy in the README. --- .github/workflows/rust.yml | 30 ++++++++++++++++++++++++++++++ CHANGELOG.md | 4 ++++ Cargo.toml | 1 + README.md | 8 ++++++++ src/secure/windows.rs | 6 ++---- 5 files changed, 45 insertions(+), 4 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index e151df3..875a751 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -63,6 +63,36 @@ jobs: run: | cargo llvm-cov test --features secure --fail-under-lines 90 + # Checks that the crate compiles with the `rust-version` of Cargo.toml, as + # recommended by the Cargo book: `cargo check` only, on one runner, with the + # other platforms type-checked through their targets (no linking needed). + msrv: + runs-on: ubuntu-latest + env: + # No Cargo.lock is committed: pick dependency versions compatible with + # the `rust-version` of Cargo.toml. + CARGO_RESOLVER_INCOMPATIBLE_RUST_VERSIONS: fallback + steps: + - name: Checkout sources + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Install rust toolchain + uses: dtolnay/rust-toolchain@02cb101ec7c40f2c49e1d9714d64511d8e1b74de # master + with: + toolchain: "1.85.0" + targets: x86_64-pc-windows-msvc, aarch64-apple-darwin + + - name: Cargo check + run: cargo check --all-targets + + - name: Cargo check (secure feature) + run: | + for target in x86_64-unknown-linux-gnu x86_64-pc-windows-msvc aarch64-apple-darwin; do + cargo check --all-targets --features secure --target "$target" + done + workflow-lint: runs-on: ubuntu-latest env: diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ec2123..bfe4a10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed ### Changed + +- Declared the minimum supported Rust version (MSRV): 1.85. It was already required by + the dependencies and is now checked by the CI. See the README for the MSRV policy. + ### Deprecated ### Removed ### Security diff --git a/Cargo.toml b/Cargo.toml index f203b13..aea91d7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,6 +3,7 @@ name = "async-walkdir" version = "2.2.0" authors = ["Ririsoft "] edition = "2021" +rust-version = "1.85" description = "Asynchronous directory traversal for Rust." license = "Apache-2.0" keywords = ["async", "walk", "directory", "recursive", "stream"] diff --git a/README.md b/README.md index da7f9b2..40335a7 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,14 @@ block_on(async { }); ``` +# Minimum supported Rust version + +The minimum supported Rust version (MSRV) is **1.85**, with every Cargo feature and on +every supported platform. It is checked by the CI. + +The MSRV may be raised in a minor release, never in a patch release. Rust versions released +during the last 12 months remain supported. + # Security: symlinks and concurrent modifications `WalkDir` does not follow symbolic links: a symlink is yielded as an entry but the walker diff --git a/src/secure/windows.rs b/src/secure/windows.rs index 4ae88c8..4d32fcc 100644 --- a/src/secure/windows.rs +++ b/src/secure/windows.rs @@ -321,10 +321,8 @@ fn parse_listing(bytes: &[u8], out: &mut VecDeque>) -> io:: let name_end = name_start.checked_add(name_len).ok_or_else(malformed)?; let name = record.get(name_start..name_end).ok_or_else(malformed)?; let name: Vec = name - .as_chunks::<2>() - .0 - .iter() - .map(|c| u16::from_ne_bytes(*c)) + .chunks_exact(2) + .map(|c| u16::from_ne_bytes([c[0], c[1]])) .collect(); if name != [u16::from(b'.')] && name != [u16::from(b'.'), u16::from(b'.')] {