Skip to content

Reported Zip Slip fix#1313

Closed
adamhathcock wants to merge 7 commits into
releasefrom
adam/zip-slip-fix
Closed

Reported Zip Slip fix#1313
adamhathcock wants to merge 7 commits into
releasefrom
adam/zip-slip-fix

Conversation

@adamhathcock
Copy link
Copy Markdown
Owner

This pull request refactors and modernizes the extraction logic in the SharpCompress library, focusing on moving extraction methods from static utility classes to extension methods on the IEntry type. It also introduces a new DirectoryManagement utility for robust path handling and security, and removes the old static ExtractionMethods classes. The main goals are to improve code organization, enhance security checks, and streamline extraction workflows.

Key changes include:

Refactoring and API Modernization

  • Migrated extraction methods such as WriteEntryToDirectory, WriteEntryToFile, and their async counterparts from the static ExtractionMethods classes to extension methods on IEntry, providing a more object-oriented and discoverable API. (src/SharpCompress/Archives/IArchiveEntryExtensions.cs, src/SharpCompress/Common/IEntryExtensions.Async.cs, [1] [2] [3] [4] [5]

  • Updated all usages in IArchiveExtensions and IAsyncArchiveExtensions to use the new extension methods, removing reliance on the old static methods. (src/SharpCompress/Archives/IArchiveEntryExtensions.cs, src/SharpCompress/Archives/IArchiveExtensions.cs, src/SharpCompress/Archives/IAsyncArchiveExtensions.cs, [1] [2] [3] [4] [5] [6]

Security and Path Handling

Code Cleanup and Removal

  • Removed the obsolete static ExtractionMethods and ExtractionMethods.Async classes, consolidating extraction logic into the new extension methods and utility class. (src/SharpCompress/Common/ExtractionMethods.cs, src/SharpCompress/Common/ExtractionMethods.Async.cs, [1] [2]

Minor Improvements

  • Cleaned up unnecessary using directives and improved XML documentation in extension classes. (src/SharpCompress/Archives/IArchiveExtensions.cs, src/SharpCompress/Archives/IAsyncArchiveExtensions.cs, [1] Fa3e9948L1, [2]

These changes make the extraction API more idiomatic, secure, and maintainable.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors extraction path handling by moving shared extraction logic onto IEntry extensions and centralizing destination-path validation in a new DirectoryManagement helper. It fits into SharpCompress’s extraction pipeline by changing the common code paths used by archive, reader, and entry extraction APIs while also adding security-focused tests.

Changes:

  • Moved extraction helpers from the old internal ExtractionMethods classes into new IEntry extension methods.
  • Added DirectoryManagement and Utility.PathComparison to normalize destination paths and enforce traversal checks.
  • Added new security tests covering Zip Slip/path traversal scenarios across sync and async archive/reader APIs.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/SharpCompress.Test/Security/ZipSlip.cs Adds a focused Zip Slip regression test for archive extraction APIs.
tests/SharpCompress.Test/Security/ExtractionPathTraversalTests.cs Adds broader sync/async traversal coverage across archive and reader entrypoints.
src/SharpCompress/Utility.cs Moves path-comparison behavior into a shared utility.
src/SharpCompress/Readers/IReaderExtensions.cs Repoints sync reader extraction to the new IEntry helpers.
src/SharpCompress/Readers/IAsyncReaderExtensions.cs Repoints async reader extraction to the new IEntry helpers.
src/SharpCompress/Common/IEntryExtensions.cs Introduces shared sync extraction logic on IEntry.
src/SharpCompress/Common/IEntryExtensions.Async.cs Introduces shared async extraction logic on IEntry.
src/SharpCompress/Common/ExtractionMethods.cs Removes the old sync extraction helper implementation.
src/SharpCompress/Common/ExtractionMethods.Async.cs Removes the old async extraction helper implementation.
src/SharpCompress/Common/DirectoryManagement.cs Adds centralized destination-directory normalization and containment checks.
src/SharpCompress/Archives/IAsyncArchiveExtensions.cs Updates async archive extraction to use the new shared core methods.
src/SharpCompress/Archives/IArchiveExtensions.cs Updates sync archive extraction to use the new shared core methods.
src/SharpCompress/Archives/IArchiveEntryExtensions.cs Repoints archive-entry extraction APIs to the new IEntry helpers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/SharpCompress/Archives/IArchiveExtensions.cs Outdated
Comment thread src/SharpCompress/Archives/IAsyncArchiveExtensions.cs
Comment thread tests/SharpCompress.Test/Security/ZipSlip.cs
@adamhathcock adamhathcock self-assigned this May 5, 2026
@adamhathcock
Copy link
Copy Markdown
Owner Author

This was somehow merged

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.

2 participants