Repository navigation
Add an opt-in secure walker resistant to symlink races - #22
Merged
Merged
Conversation
ririsoft
force-pushed
the
secure-walkdir
branch
from
September 29, 2026 08:31
b5c11ef to
2e7fad2
Compare
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.
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.
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.
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.
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.
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.
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.
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.
ririsoft
force-pushed
the
secure-walkdir
branch
from
September 29, 2026 10:00
0c3e09e to
c280f22
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
WalkDirchecks an entry's type without following symlinks, then opens the directory again by path, which does follow them. A concurrent process able to write to the walked tree can swap a directory for a symlink between the two calls and make the walk escape the root. This is a TOCTOU race of the same class as CVE-2022-21658 (std::fs::remove_dir_all) and BurntSushi/walkdir#209.This PR:
secureCargo feature (off by default, Unix and Windows only) providingasync_walkdir::secure::WalkDir, which walks with directory handles:openatrelative to the parent fd withO_NOFOLLOW | O_DIRECTORY(viarustix, nounsafe).NtCreateFilerelative to the parent handle withFILE_OPEN_REPARSE_POINT, then refuses symlinks and junctions by checking the opened handle (viawindows-sys). This is the only module withunsafe: the crate root goes fromforbidtodeny(unsafe_code), and eachunsafeblock has a// SAFETY:comment.open()and the parent handle (parent_fd/parent_handle). The crate owns itsFileTypeandMetadatatypes, so norustixorwindows-systype appears in the public API.secure, tests and coverage withsecure,macos-latestadded to the matrix.The existing
WalkDiris unchanged, so this is a semver-minor change. No version bump in this PR.Review notes
The Windows backend was only type-checked locally; this CI run is the first real execution. Points to confirm on Windows:
NtCreateFile;EaSizeinFileIdBothDirectoryInforecords;