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.

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)

…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

zhw2101024 · 2026-03-23T09:52:22Z

@abendrothj Would you please have a look at the issue I'm having? It seems the ancestors_mode_directories_with_file test only passed with the root user on my system, and unsure if my "fix" is correct.

Something changed in 0.7.0 re. what happens when boulder builds some packages, that makes boulder record directories as part of the package, which can in turn cause conflicts (notably seen with inputplumber). To avoid this becoming an issue, and to give us time to investigate further, downgrade to 0.6.0 for now. Signed-off-by: Rune Morling <ermo@aerynos.com>

## Summary Fixes #11469 `install -D` was replacing pre-existing symlinks in the destination path with real directories instead of following them. This broke any workflow where part of the install prefix is a symlink; including BOSH deployments, Homebrew, Nix, stow, and any `make install` targeting a symlinked prefix. **Reproduction (from the issue):** ```sh mkdir -p /tmp/target ln -s /tmp/target /tmp/link echo hello > /tmp/file.txt install -D -m 644 /tmp/file.txt /tmp/link/subdir/file.txt # GNU coreutils 8.32: /tmp/link stays a symlink, file lands in /tmp/target/subdir/file.txt # uutils 0.7.0: /tmp/link is replaced with a real directory — wrong ``` ## Root cause PR #10140 introduced `create_dir_all_safe()` in `safe_traversal.rs` to prevent TOCTOU symlink race conditions. The fix was correct in intent but too aggressive: `open_or_create_subdir()` unconditionally unlinked and recreated any symlink it encountered, including pre-existing legitimate ones. ## Changes **`src/uucore/src/lib/features/safe_traversal.rs`** - `open_or_create_subdir`: when `stat_at` returns `S_IFLNK`, call `open_subdir(Follow)` instead of `unlink_at + mkdir_at`. The `O_DIRECTORY` flag already in `open_subdir` means dangling or non-directory symlinks still return an error cleanly. - `find_existing_ancestor`: switch from `fs::symlink_metadata` to `fs::metadata` so that a symlink-to-directory is recognised as an existing ancestor rather than a component to recreate (this was already the stated intent in the function's doc comment). **`src/uu/install/src/install.rs`** - Align the `dir_exists` check and the `DirFd::open` call to also follow symlinks, consistent with the above. **`tests/by-util/test_install.rs`** - Update the two tests added by #10140 — they were asserting the buggy behavior (symlink replaced). Flip the assertions to document the correct GNU behavior. - Add `test_install_d_follows_symlink_prefix` as a direct regression test for the issue's reproduction case. ## TOCTOU / security note The true TOCTOU race (a symlink *injected during the operation* into a not-yet-existing path component) is still blocked: `mkdirat` fails with `EEXIST` if an attacker creates a symlink between `stat_at` returning `ENOENT` and our `mkdir_at`. Newly-created directories are still opened with `O_NOFOLLOW`. What changes is that *pre-existing* symlinks are now followed — which is exactly what GNU coreutils 8.32 does. The previous behavior was stricter than GNU in this regard.

## Summary Fixes uutils#11469 `install -D` was replacing pre-existing symlinks in the destination path with real directories instead of following them. This broke any workflow where part of the install prefix is a symlink; including BOSH deployments, Homebrew, Nix, stow, and any `make install` targeting a symlinked prefix. **Reproduction (from the issue):** ```sh mkdir -p /tmp/target ln -s /tmp/target /tmp/link echo hello > /tmp/file.txt install -D -m 644 /tmp/file.txt /tmp/link/subdir/file.txt # GNU coreutils 8.32: /tmp/link stays a symlink, file lands in /tmp/target/subdir/file.txt # uutils 0.7.0: /tmp/link is replaced with a real directory — wrong ``` ## Root cause PR uutils#10140 introduced `create_dir_all_safe()` in `safe_traversal.rs` to prevent TOCTOU symlink race conditions. The fix was correct in intent but too aggressive: `open_or_create_subdir()` unconditionally unlinked and recreated any symlink it encountered, including pre-existing legitimate ones. ## Changes **`src/uucore/src/lib/features/safe_traversal.rs`** - `open_or_create_subdir`: when `stat_at` returns `S_IFLNK`, call `open_subdir(Follow)` instead of `unlink_at + mkdir_at`. The `O_DIRECTORY` flag already in `open_subdir` means dangling or non-directory symlinks still return an error cleanly. - `find_existing_ancestor`: switch from `fs::symlink_metadata` to `fs::metadata` so that a symlink-to-directory is recognised as an existing ancestor rather than a component to recreate (this was already the stated intent in the function's doc comment). **`src/uu/install/src/install.rs`** - Align the `dir_exists` check and the `DirFd::open` call to also follow symlinks, consistent with the above. **`tests/by-util/test_install.rs`** - Update the two tests added by uutils#10140 — they were asserting the buggy behavior (symlink replaced). Flip the assertions to document the correct GNU behavior. - Add `test_install_d_follows_symlink_prefix` as a direct regression test for the issue's reproduction case. ## TOCTOU / security note The true TOCTOU race (a symlink *injected during the operation* into a not-yet-existing path component) is still blocked: `mkdirat` fails with `EEXIST` if an attacker creates a symlink between `stat_at` returning `ENOENT` and our `mkdir_at`. Newly-created directories are still opened with `O_NOFOLLOW`. What changes is that *pre-existing* symlinks are now followed — which is exactly what GNU coreutils 8.32 does. The previous behavior was stricter than GNU in this regard.

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