fix(install): follow symlink components in destination path with -D#11505
fix(install): follow symlink components in destination path with -D#11505abendrothj wants to merge 2 commits intouutils:mainfrom
Conversation
9c7f91a to
4fc3ac8
Compare
|
GNU testsuite comparison: |
Merging this PR will degrade performance by 23.82%
Performance Changes
Comparing Footnotes
|
ebc0ba6 to
cd070c4
Compare
|
GNU testsuite comparison: |
install -D was replacing pre-existing symlinks in the destination path with real directories instead of following them, breaking any workflow where part of the install prefix is a symlink (BOSH, Homebrew, Nix, stow, `make install` with a symlinked prefix).
cd070c4 to
ed62194
Compare
|
GNU testsuite comparison: |
|
The test fails in the mkfifo test, anyway hope this PR can be merged, and the related issue not left to the next released uutils. |
agreed. seems like flakiness, since mkfifo is unrelated. |
|
GNU testsuite comparison: |
Summary
Fixes #11469
install -Dwas 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 anymake installtargeting a symlinked prefix.Reproduction (from the issue):
Root cause
PR #10140 introduced
create_dir_all_safe()insafe_traversal.rsto 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.rsopen_or_create_subdir: whenstat_atreturnsS_IFLNK, callopen_subdir(Follow)instead ofunlink_at + mkdir_at. TheO_DIRECTORYflag already inopen_subdirmeans dangling or non-directory symlinks still return an error cleanly.find_existing_ancestor: switch fromfs::symlink_metadatatofs::metadataso 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.rsdir_existscheck and theDirFd::opencall to also follow symlinks, consistent with the above.tests/by-util/test_install.rstest_install_d_follows_symlink_prefixas 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:
mkdiratfails withEEXISTif an attacker creates a symlink betweenstat_atreturningENOENTand ourmkdir_at. Newly-created directories are still opened withO_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.