Skip to content

fix: reject archive entries that would extract outside the destination - #211

Open
kim-em wants to merge 3 commits into
leanprover:masterfrom
kim-em:harden-archive-extraction
Open

kim-em wants to merge 3 commits into
leanprover:masterfrom
kim-em:harden-archive-extraction

Conversation

@kim-em

@kim-em kim-em commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

This PR hardens toolchain archive extraction so that entries cannot be written outside the destination directory. After the top-level directory is stripped, tar and zip entry paths may only consist of plain file names; components such as .., roots, drive prefixes, or names that Windows would reinterpret are rejected with an error. Tar hard links are rejected, and symbolic links are accepted only if their target stays within the link's own directory, which covers the relative library links in current Linux releases. It also updates tar to 0.4.46 for its own symlink handling fix.

🤖 Prepared with Claude Code

Validate paths of tar and zip entries after stripping the top-level
directory, accepting only plain file names. Reject hard links in tar
archives and accept symbolic links only if their target stays within
the link's own directory. Update tar to 0.4.46.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Kha

Kha commented Oct 5, 2026

Copy link
Copy Markdown
Member

tar claims to already do:

paths containing .. are rejected, and symlink targets within the archive are validated before use.

@hummbl-dev hummbl-dev left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

The patch strips the archive root, then rebuilds tar and zip paths from plain components and writes through that full_path (entry.unpack(&full_path) / File::create), so a lexical .., drive prefix, or absolute remainder no longer survives Path::join. Hard links are rejected before tar 0.4.46's Entry::unpack, which would otherwise fs::hard_link the target against the process cwd. The hole is symbolic links whose path collapses to the destination itself: the target check never joins the link location, and a later entry is created through that link.

Issue counts by severity

  • bugs: 1
  • suggestions: 0
  • nits: 0

Comment thread src/elan-dist/src/component/package.rs Resolved
kim-em and others added 2 commits October 6, 2026 12:51
An entry such as `pkg` or `pkg/.` strips to an empty path and would be
unpacked at the destination itself; a symlink there is resolved relative
to the destination's parent. Allow only directories at that path, in
both tar and zip archives.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@kim-em

kim-em commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Claude+Codex say:

@Kha, that guarantee applies to Archive::unpack and Entry::unpack_in, which derive the output path from the entry path. elan strips the top-level directory, constructs the output path, and calls Entry::unpack(full_path). Its documentation says it “does not prevent writes outside dst” and recommends unpack_in for untrusted archives.

Entry::unpack writes to the supplied path and resolves relative hard-link targets against the process working directory. unpack_in rejects .. while constructing the path and verifies that each entry’s parent still resolves inside dst, including after an earlier symlink.

On master, pkg/../../x is joined to the destination and written outside it. This PR requires plain path components, confines symlink targets to the link’s directory or below, and rejects hard links.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants