fix(cp): open source files with O_NOFOLLOW to prevent TOCTOU symlink swap#11995
Open
mattsu2020 wants to merge 4 commits intouutils:mainfrom
Open
fix(cp): open source files with O_NOFOLLOW to prevent TOCTOU symlink swap#11995mattsu2020 wants to merge 4 commits intouutils:mainfrom
mattsu2020 wants to merge 4 commits intouutils:mainfrom
Conversation
…swap (uutils#10017) cp previously checked whether a source is a symlink using lstat-style metadata, but then opened the source by path without O_NOFOLLOW. An attacker who could mutate the source tree concurrently could replace a regular file with a symlink between the check and the open, causing cp to copy the symlink target's contents. This fix adds O_NOFOLLOW when opening source files in all platform modules (linux, macos, other_unix) when symlink dereferencing is not requested. When -L/--dereference is used, symlinks are still followed as the user intended.
|
GNU testsuite comparison: |
- Adjusted line breaks and formatting in `src/uu/cp/src/platform/linux.rs` - Adjusted line breaks and formatting in `src/uu/cp/src/platform/macos.rs` - Updated spell-checker ignore list to include `ELOOP` and `dstdir` - Reformatted test code in `tests/by-util/test_cp.rs` for consistency
Contributor
|
Is copy_stream() used for normal cross-device copy after this PR? |
The function is now used for both stream and regular file copies after the O_NOFOLLOW fix, so the name should reflect its general purpose.
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
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
Fixes #10017
cpwas vulnerable to a TOCTOU (Time-of-Check-Time-of-Use) attack. It checked whether a source is a symlink usinglstat-style metadata, but then opened the source by path withoutO_NOFOLLOW. An attacker who could mutate the source tree concurrently could replace a regular file with a symlink between the check and the open, causingcpto copy the symlink target's contents.O_NOFOLLOWviaOpenOptions::custom_flags()when symlink dereferencing is not requested (-Lnot specified). When-L/--dereferenceis used, symlinks are still followed as intended.std::fs::copy()fallbacks replaced withopen_source()+buf_copy::copy_stream()to maintainO_NOFOLLOWprotection throughout the copy pipeline.Changes
src/uu/cp/src/platform/linux.rs— Addedopen_source()helper andcopy_source_to_dest(). Threadedfollow_symlinksthrough all internal functions (clone,sparse_copy,copy_stream,handle_reflink_*, etc.)src/uu/cp/src/platform/macos.rs— Addedopen_source()helper. ReplacedFile::open()andfs::copy()withO_NOFOLLOW-aware alternatives.src/uu/cp/src/platform/other_unix.rs— Same as macOS.src/uu/cp/src/platform/other.rs— Signature update only (non-Unix,O_NOFOLLOWnot available).src/uu/cp/src/cp.rs— Threadedsource_in_command_linethroughcopy_helperto derivedereferenceflag forcopy_on_write.GNU cp parity
Test plan
cargo build -p uu_cp— compiles successfullycargo test -p uu_cp— existing unit tests passtests/by-util/test_cp.rs:copy_regular_file_works— basic copy still workscopy_dereference_follows_symlink—-Lfollows symlinkscopy_symlink_as_link_no_dereference—-Pcopies symlink as linkcopy_reflink_never_regular_file—--reflink=neverpath worksrecursive_copy_regular_files— recursive copy workscopy_no_dereference_regular_file—-Pwith regular file works