Skip to content

fix(core)!: stop trusting pre-existing symlinks in extraction destination - #816

Merged
bug-ops merged 3 commits into
mainfrom
hotfix/dircache-symlink-escape
Nov 2, 2026
Merged

bug-ops merged 3 commits into
mainfrom
hotfix/dircache-symlink-escape

Conversation

@bug-ops

@bug-ops bug-ops commented Nov 2, 2026

Copy link
Copy Markdown
Owner

Summary

Extraction previously cached directories it had "created" and trusted them for the rest of the run. A symlink already present in the destination was treated as such a directory, so entries could be written outside the destination.

  • Replace DirCache with DirWalker, which runs lstat on every component below the destination for each entry write.
  • Pre-existing symlinks are refused by default. With symlinks allowed they are accepted only when they resolve to a directory inside the destination.
  • SafePath no longer trusts the parent; the parent is canonicalized on every entry except when it is the destination root.
  • On non-Unix targets, writing through a symlinked final component is refused.
  • Applies to TAR, ZIP and 7z, including the atomic and fresh-destination paths.

Behavior change

By default extraction now fails if any component below the destination is a symlink, even one pointing inside the destination. --allow-symlinks restores that for contained links. A symlink as the destination root itself is still accepted.

Breaking changes

  • EntryValidator::validate_entry no longer takes a dir_cache argument.
  • ValidationContext no longer has a lifetime parameter.

Known limitations

  • The check is path-based, so a local attacker with write access to the destination during extraction can still race between lstat and the write. Closing this fully requires openat from a pinned directory fd (follow-up).
  • Possible overhead on deeply nested archives from the extra per-entry lstat and parent canonicalization; benchmarks were noisy and need a re-run on an idle machine.
  • The non-Unix symlink refusal is verified by compilation only.

Test plan

  • New integration tests in tests/preexisting_dest_symlink.rs for TAR, ZIP and 7z: directory entry through a link, intermediate component, no directory entries, --force overwrite, re-extraction, swap during extraction, --allow-symlinks with an escaping link.
  • Full CI suite: fmt, clippy, nextest, doc tests, rustdoc, cargo deny.
  • Live checks with the CLI against crafted archives, plus Python and Node bindings.

@github-actions github-actions Bot added core Changes to exarch-core docs Documentation tests Test changes benches Benchmark changes dependencies Dependency updates labels Nov 2, 2026
…tion

Replace the directory cache with a per-component lstat walker so that every
parent component below the destination is verified on each entry write.
Pre-existing symlinks are refused by default; with symlinks allowed they are
accepted only when they resolve inside the destination.

Skip parent canonicalization when the parent is the destination root and
refuse symlinked final components on non-Unix targets.

BREAKING CHANGE: EntryValidator::validate_entry no longer takes a dir_cache
argument, and ValidationContext no longer carries a lifetime parameter.
@bug-ops
bug-ops force-pushed the hotfix/dircache-symlink-escape branch from a87d494 to 21ebe62 Compare November 2, 2026 12:41
@bug-ops
bug-ops enabled auto-merge (squash) November 2, 2026 12:41
@bug-ops
bug-ops merged commit 9c6a986 into main Nov 2, 2026
33 checks passed
@bug-ops
bug-ops deleted the hotfix/dircache-symlink-escape branch November 2, 2026 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

benches Benchmark changes core Changes to exarch-core dependencies Dependency updates docs Documentation tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant