Skip to content

Update TAR guidance for untrusted archives#54670

Open
mangod9 wants to merge 4 commits into
dotnet:mainfrom
mangod9:copilot/tar-docs-security
Open

Update TAR guidance for untrusted archives#54670
mangod9 wants to merge 4 commits into
dotnet:mainfrom
mangod9:copilot/tar-docs-security

Conversation

@mangod9

@mangod9 mangod9 commented Jul 8, 2026

Copy link
Copy Markdown
Member

Updating the ZIP/TAR best-practices docs with security guidance for TAR API use: destination boundary validation, PAX extended attributes, and link names.


Internal previews

📄 File 🔗 Preview link
docs/standard/io/zip-tar-best-practices.md Best practices for working with ZIP and TAR archives in .NET

…ributes

Adds security guidance to the ZIP/TAR best-practices article for handling
untrusted TAR archives:
- A note that destination boundary validation assumes the destination is a
  real directory the app controls; a junction/symlink destination (or parent)
  causes extraction to follow the link, even with TarFile.ExtractToDirectory.
- A new section on PAX extended attributes and link names as untrusted,
  archive-supplied input that can override header fields.
- Updated the untrusted-metadata checklist item to include link names and
  PAX extended attributes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 8, 2026 05:39
@mangod9 mangod9 requested review from a team and adegeo as code owners July 8, 2026 05:39
@dotnetrepoman dotnetrepoman Bot added this to the July 2026 milestone Jul 8, 2026
@mangod9 mangod9 changed the title Update TAR security guidance for untrusted archives Update TAR guidance for untrusted archives Jul 8, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates the archive security best-practices guidance to cover TAR-specific risks when extracting untrusted archives, complementing the existing ZIP guidance.

Changes:

  • Adds a note clarifying the limits of destination boundary validation when the destination path contains junctions or symlinks.
  • Documents TAR PAX extended attributes and TAR link names as untrusted metadata that requires validation and limits.
  • Updates the best-practices checklist to explicitly include TAR link names and PAX extended attributes.

Comment thread docs/standard/io/zip-tar-best-practices.md Outdated
Comment thread docs/standard/io/zip-tar-best-practices.md Outdated
Comment thread docs/standard/io/zip-tar-best-practices.md Outdated
Co-authored-by: Genevieve Warren <24882762+gewarren@users.noreply.github.com>
@mangod9

mangod9 commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

@copilot can you please apply the review suggested text changes?

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- The trailing directory separator on `fullDestDir` is critical—without it, a path like `/safe-dir-evil/file` would incorrectly match `/safe-dir`.

> [!NOTE]
> Boundary validation assumes the destination directory itself is a real directory you control. If the destination directory or one of its parents is already a junction or symbolic link, extraction follows that link and writes to the link's target—even with <xref:System.Formats.Tar.TarFile.ExtractToDirectory*?displayProperty=nameWithType>, which otherwise rejects entries and link targets that escape the destination. When handling untrusted archives, extract into a fresh, application-controlled directory rather than a reused or externally writable location.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When handling untrusted archives, extract into a fresh, application-controlled directory rather than a reused or externally writable location.

This is inconsistent with line 44 in this same document, which says that ExtractToDirectory is meant only for trusted archives.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

updated the content to not include ExtractToDirectory

…chive note

The note lives in the streaming/untrusted-input section, but referenced
TarFile.ExtractToDirectory, which the article frames as a trusted-input
convenience API. Reword to keep the note focused on manual boundary
validation and avoid the inconsistency.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

4 participants