fix(install): prevent symlink race condition in install -D (fixes #10013) #10140

abendrothj · 2026-01-09T12:00:19Z

This PR fixes a TOCTOU (Time-of-Check-Time-of-Use) race condition in the install -D command where an attacker could replace a directory component with a symlink between directory creation and file installation, redirecting writes to an arbitrary location.

Changes

Added mkdir_at() and open_file_at() methods to DirFd for safe operations
Added open_no_follow() to prevent following symlinks when opening directories
Added create_dir_all_safe() function that uses directory file descriptors
Modified install -D to use safe traversal functions instead of pathname-based ops
Added copy_file_safe() function for safe file copying using directory fds
Added tests to verify the fix prevents symlink race conditions

How the Fix Works

The fix prevents the race condition by:

Using directory file descriptors (mkdirat/openat) instead of pathnames
Keeping directory fds open throughout the operation
Detecting and removing symlinks before directory creation
Anchoring all file operations to directory file descriptors

This eliminates the race window by ensuring all critical operations use directory file descriptors that cannot be replaced by symlinks.

Testing

All existing tests pass (3823 passed, 0 failed)
Added 2 new tests that verify the race condition is prevented
Tests reproduce the exact scenario from issue Symlink race in install -D directory creation #10013

Fixes #10013

github-actions · 2026-01-09T13:10:29Z

GNU testsuite comparison:

GNU test failed: tests/install/basic-1. tests/install/basic-1 is passing on 'main'. Maybe you have to rebase?

github-actions · 2026-01-15T09:21:21Z

GNU testsuite comparison:

GNU test failed: tests/install/basic-1. tests/install/basic-1 is passing on 'main'. Maybe you have to rebase?

codspeed-hq · 2026-01-15T12:20:53Z

Merging this PR will not alter performance

✅ 284 untouched benchmarks
⏩ 38 skipped benchmarks¹

_{Comparing abendrothj:fix/install-symlink-race-condition-10013 (89c2d2b) with main (bb91a5b)}

38 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

abendrothj · 2026-01-17T08:45:23Z

The 3.46% performance regression is an acceptable trade-off for fixing the symlink race condition vulnerability. The safe directory traversal using file descriptors adds unavoidable overhead, but prevents potential security exploits.

github-actions · 2026-01-17T09:27:30Z

GNU testsuite comparison:

GNU test failed: tests/tail/follow-name. tests/tail/follow-name is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/shuf/shuf-reservoir (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/sort/sort-stale-thread-mem (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/tty/tty-eof (passes in this run but fails in the 'main' branch)

sylvestre · 2026-01-17T17:11:13Z

sorry but could you please rebase it ? thanks

sylvestre · 2026-01-18T10:32:04Z

The 3.46% performance regression is an acceptable trade-off for fixing the symlink race condition vulnerability. The safe directory traversal using file descriptors adds unavoidable overhead, but prevents potential security exploits.

agreed

sylvestre · 2026-01-19T07:48:40Z

some conflicts, sorry

abendrothj · 2026-01-20T03:14:31Z

There, that should resolve the conflicts :)

github-actions · 2026-01-20T03:32:10Z

GNU testsuite comparison:

Skip an intermittent issue tests/tail/follow-name (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/dd/stderr is no longer failing!
Congrats! The gnu test tests/tac/tac-2-nonseekable is no longer failing!
Congrats! The gnu test tests/tail/follow-stdin is no longer failing!

abendrothj · 2026-01-20T03:44:34Z

The sort regression is benchmark noise - this PR only touches install and safe_traversal, which sort doesn't use. Seems like the android emulator failure is unrelated too.

src/uu/install/src/install.rs

src/uucore/src/lib/features/safe_traversal.rs

tests/by-util/test_install.rs

src/uucore/src/lib/features/safe_traversal.rs

src/uu/install/src/install.rs

src/uucore/src/lib/features/safe_traversal.rs

sylvestre · 2026-01-20T10:31:44Z

This is a lot of added complexity and I think you could have a bit more polish before submitting for review :/

abendrothj · 2026-01-20T11:29:47Z

This is a lot of added complexity and I think you could have a bit more polish before submitting for review :/

Yeah that was rough, won't happen again

github-actions · 2026-01-21T02:06:07Z

GNU testsuite comparison:

Skip an intermittent issue tests/shuf/shuf-reservoir (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/sort/sort-stale-thread-mem (fails in this run but passes in the 'main' branch)

github-actions · 2026-01-21T07:27:08Z

GNU testsuite comparison:

Congrats! The gnu test tests/factor/t10 is no longer failing!
Congrats! The gnu test tests/factor/t26 is no longer failing!
Congrats! The gnu test tests/factor/t27 is no longer failing!
Congrats! The gnu test tests/factor/t28 is no longer failing!
Congrats! The gnu test tests/factor/t29 is no longer failing!
Congrats! The gnu test tests/factor/t30 is no longer failing!
Congrats! The gnu test tests/factor/t31 is no longer failing!
Congrats! The gnu test tests/factor/t32 is no longer failing!
Congrats! The gnu test tests/factor/t36 is no longer failing!

github-actions · 2026-01-21T12:10:19Z

GNU testsuite comparison:

Skipping an intermittent issue tests/shuf/shuf-reservoir (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/sort/sort-stale-thread-mem (passes in this run but fails in the 'main' branch)

github-actions · 2026-01-21T14:19:36Z

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)

sylvestre · 2026-02-04T21:48:17Z

src/uu/install/src/install.rs

+/// Copy a file using directory file descriptor for safe traversal.
+/// This prevents symlink race conditions when creating target files.
+#[cfg(unix)]
+fn copy_file_safe(from: &Path, to_parent_fd: &DirFd, to_filename: &std::ffi::OsStr) -> UResult<()> {


some code duplication with copy_file it seems

These can't be easily consolidated as they have different semantics:
copy_file_safe: fd-based (open_file_at), truncates existing files
copy_file: path-based (create_new), removes first, includes directory override check
Both are used: copy_file_safe at line 774 (safe -D path), copy_file at line 1148 (regular installs). Consolidating would require multiple match branches and obscure the distinct behaviors.

github-actions · 2026-02-06T08:11:19Z

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/usage_vs_getopt (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/shuf/shuf-reservoir (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/sort/sort-stale-thread-mem (passes in this run but fails in the 'main' branch)

abendrothj · 2026-02-06T10:21:19Z

Failures seem unrelated to my PR and it looks like theres a decision to make on upstream- cargo-deny: time v0.3.47 requires Rust 1.88.0 but the project's MSRV is 1.85.0. This will affect all PRs and main too. Upstream needs to either bump MSRV or add an ignore for it, definitely out of my bounds in this PR.

github-actions · 2026-02-06T10:28:43Z

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/usage_vs_getopt (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/shuf/shuf-reservoir (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/sort/sort-stale-thread-mem (passes in this run but fails in the 'main' branch)

…ils#10013) This commit fixes a TOCTOU (Time-of-Check-Time-of-Use) race condition in the install -D command where an attacker could replace a directory component with a symlink between directory creation and file installation, redirecting writes to an arbitrary location. Changes: - Added mkdir_at() and open_file_at() methods to DirFd for safe operations - Added open_no_follow() to prevent following symlinks when opening directories - Added create_dir_all_safe() function that uses directory file descriptors - Modified install -D to use safe traversal functions instead of pathname-based ops - Added copy_file_safe() function for safe file copying using directory fds - Added tests to verify the fix prevents symlink race conditions The fix works by: 1. Using directory file descriptors (mkdirat/openat) instead of pathnames 2. Keeping directory fds open throughout the operation 3. Detecting and removing symlinks before directory creation 4. Anchoring all file operations to directory file descriptors This eliminates the race window by ensuring all critical operations use directory file descriptors that cannot be replaced by symlinks.

… and boolean simplifications

… list

…x context setting to Linux platforms This commit updates the conditional compilation for SELinux context settings in the install module to only apply when the target OS is Linux. This change ensures that SELinux-related functionality is not erroneously included on non-Linux platforms, improving compatibility and preventing potential issues. Changes: - Modified `#[cfg(feature = "selinux")]` to `#[cfg(all(feature = "selinux", target_os = "linux"))]` in multiple locations to enforce the OS-specific behavior.

…s in tests

…s a file When -t is used with -D and the target exists as a file (not a directory), we should fail immediately without printing verbose directory creation messages. This matches GNU behavior and fixes the failing GNU test tests/install/basic-1.

…ile_safe

- Replace std::io::copy() with copy_stream() for consistency and splice optimization - Consolidate DirFd::open() and open_subdir() to take follow_symlinks parameter - Extract finalize_installed_file() helper to reduce code duplication - Add documentation for security behavior (symlink removal) and mode handling - Clean up verbose comments

github-actions · 2026-02-11T21:24:49Z

GNU testsuite comparison:

GNU test failed: tests/pr/bounded-memory. tests/pr/bounded-memory is passing on 'main'. Maybe you have to rebase?

sylvestre · 2026-02-11T22:11:26Z

it looks great, i will wait for the ci to finish

abendrothj force-pushed the fix/install-symlink-race-condition-10013 branch from fc7eade to 0b91865 Compare January 15, 2026 09:10

abendrothj force-pushed the fix/install-symlink-race-condition-10013 branch 2 times, most recently from fe7aec2 to 4fb0779 Compare January 19, 2026 07:29

abendrothj force-pushed the fix/install-symlink-race-condition-10013 branch from 4fb0779 to f95bed7 Compare January 20, 2026 03:12