Skip to content

Reject symlinks and hardlinks on Windows when opening .part files - #39

Merged
gistrec merged 2 commits into
masterfrom
fix/windows-part-symlink-guard
Jul 6, 2026
Merged

Reject symlinks and hardlinks on Windows when opening .part files#39
gistrec merged 2 commits into
masterfrom
fix/windows-part-symlink-guard

Conversation

@gistrec

@gistrec gistrec commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Problem

The POSIX receiver already refuses a pre-planted symlink (O_NOFOLLOW) and hardlink (st_nlink == 1) at the predictable <name>.part path (#32). The Windows path had no equivalent guard: it opened .part with CREATE_ALWAYS and no reparse-point handling. A local attacker who can write the receiver's working directory could therefore plant a reparse point or hardlink at the predictable <name>.part name and:

  • redirect the receiver's writes to a victim file, and
  • have CREATE_ALWAYS truncate the victim first.

Fix

openPartFile on Windows now mirrors the POSIX guard:

  • CREATE_ALWAYSOPEN_ALWAYS — never truncates an existing file before it is vetted.
  • Open with FILE_FLAG_OPEN_REPARSE_POINT — opens the link itself, not its target.
  • isLoneRegularFile(HANDLE) via GetFileInformationByHandle rejects a reparse point, a directory, or any file with nNumberOfLinks != 1, before any write — covering both fresh and --resume opens.

The check runs on the same handle used for writes, so there is no TOCTOU window between vetting and use.

Tests

  • New run_symlink_part_test e2e case, mirroring the existing hardlink test: plants a symlink at <name>.part, then asserts the receiver exits non-zero and leaves the victim byte-for-byte intact.
  • Full e2e suite + unit tests pass locally on macOS.

The Windows e2e job is skipped in CI (Winsock SO_REUSEADDR), so the new test pins the POSIX half of the guarantee; the Windows guard itself is exercised by the windows-latest compile job.

Notes

  • README already documents this guarantee platform-agnostically (updated in [Security review][L3] Reject pre-planted hardlinks when opening .part files #32) — this PR makes that statement true on Windows.
  • The FILE_ATTRIBUTE_DIRECTORY bit in isLoneRegularFile is defence in depth (CreateFileA without FILE_FLAG_BACKUP_SEMANTICS won't return a directory handle); a comment now explains why it stays.

gistrec added 2 commits July 6, 2026 14:30
The POSIX path refused a pre-planted symlink (O_NOFOLLOW) and a hardlink
(st_nlink == 1) at the .part path, but the Windows path opened it with
CREATE_ALWAYS and no reparse-point handling. A pre-planted reparse point
or hardlink therefore redirected the receiver's writes to a victim file,
and CREATE_ALWAYS truncated it first.

Open with OPEN_ALWAYS (never truncates) and FILE_FLAG_OPEN_REPARSE_POINT
(the link itself, not its target), then reject reparse points, directories
and any file with more than one link via GetFileInformationByHandle - the
Win32 counterpart to the POSIX guard, covering both fresh and --resume opens.
Mirror the existing hardlink test: plant a symlink at the predictable
<name>.part and assert the receiver refuses (non-zero exit) and leaves the
victim byte-for-byte intact. O_NOFOLLOW (POSIX) and FILE_FLAG_OPEN_REPARSE_POINT
+ GetFileInformationByHandle (Windows) both refuse it; this pins that against
regressions. The Windows e2e job is skipped in CI (Winsock SO_REUSEADDR), so
the test covers the POSIX half while the compile job covers the Windows guard.

Also expand the isLoneRegularFile(HANDLE) comment to explain that the DIRECTORY
bit is defence in depth - CreateFileA without FILE_FLAG_BACKUP_SEMANTICS won't
return a directory handle, but rejecting it keeps the guard honest if that
ever changes.
@gistrec gistrec added bug Something isn't working security Security-related fix or vulnerability labels Jul 6, 2026
@gistrec
gistrec merged commit 5ed9071 into master Jul 6, 2026
6 checks passed
@gistrec
gistrec deleted the fix/windows-part-symlink-guard branch July 6, 2026 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working security Security-related fix or vulnerability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant