Skip to content

fix(utils): harden zip extraction#495

Merged
Borda merged 6 commits into
developfrom
fix/downloader
Jul 9, 2026
Merged

fix(utils): harden zip extraction#495
Borda merged 6 commits into
developfrom
fix/downloader

Conversation

@Borda

@Borda Borda commented Jul 9, 2026

Copy link
Copy Markdown
Member

This pull request adds security checks to ZIP extraction to prevent directory traversal attacks and introduces comprehensive tests to verify the new behavior. The most important changes are:

Security improvements to ZIP extraction:

  • Added a new function _validate_zip_member_path in downloader.py to ensure that ZIP archive members cannot extract files outside the intended output directory, raising a ValueError if an unsafe path is detected.
  • Updated _extract_zip to validate all member paths before extraction and to document the new error behavior in its docstring.

Testing for ZIP extraction safety:

  • Added test_extract_zip_extracts_safe_members to verify that safe ZIP members are extracted correctly into the output directory.
  • Added test_extract_zip_rejects_unsafe_members (with parameterization) to assert that unsafe ZIP members (such as those with .. or absolute paths) are rejected and that no files are extracted in these cases.
  • Introduced a helper function _write_zip for creating test ZIP archives with arbitrary members.

resolves #477

- Validate ZIP members before extraction to reject absolute paths, traversal segments, and empty member names.
- Add regression coverage for safe archives and unsafe member names so the extraction guard stays enforced.

---

Co-authored-by: Codex <codex@openai.com>
@Borda Borda requested a review from Copilot July 9, 2026 16:19
@Borda Borda requested a review from SkalskiP as a code owner July 9, 2026 16:19

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

This PR hardens the internal ZIP extraction utility (trackers.utils.downloader._extract_zip) against Zip Slip-style directory traversal by validating ZIP member paths before extraction, and adds targeted tests to confirm both safe extraction and rejection of unsafe member paths.

Changes:

  • Added _validate_zip_member_path to reject empty, absolute, or traversal member names that would resolve outside the output directory.
  • Updated _extract_zip to validate all members before calling extractall, and documented the new ValueError behavior.
  • Added new unit tests covering safe extraction and parameterized unsafe member rejection with “no partial extraction” assertions.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/trackers/utils/downloader.py Adds path validation for ZIP members and enforces validation prior to extraction to prevent directory traversal.
tests/utils/test_downloader.py Introduces helper ZIP writer and tests for safe extraction plus rejection of unsafe member paths.

- Anchor ZIP extraction to an opened output directory so a path swap cannot redirect writes.

- Expand downloader tests with outside-path assertions, empty-member coverage, Windows-style member coverage, and a path-swap regression.

---

Co-authored-by: Codex <codex@openai.com>
@Borda

Borda commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

Measured on this machine:

  • Fixed per-archive overhead

    • 0 files, 0 depth, 0-byte payload
    • plain extractall: 0.03 ms
    • safe extractor: 0.07 ms
    • extra cost: ~0.03 ms per archive
  • Per-file overhead

    • 1 KiB files, depth 1
    • observed safe-extractor slope: ~0.090 ms/file
    • overhead at 100 files: 1.59 ms vs plain extractall
  • Per-directory-depth overhead

    • 20 files, 1 KiB each
    • observed safe-extractor slope: ~0.282 ms/depth-level
    • overhead at depth 8: 2.05 ms vs plain extractall
  • Size sensitivity

    • 20 files, depth 1
    • 1 KiB: overhead ~0.36 ms
    • 64 KiB: overhead ~1.20 ms
    • 512 KiB: overhead ~7.97 ms

Takeaway:

  • The safety cost is not constant.

  • It scales with:

    • number of files
    • directory depth
    • payload size as well, in this implementation
  • The validation itself is cheap; most of the overhead comes from the fd-anchored extraction path and Python-level copying.

Borda and others added 4 commits July 9, 2026 21:20
- Keep the Unix fd-anchored no-follow extractor for symlink-safe containment on platforms that support dir_fd.

- Add a Windows fallback plus a skipped Unix-only swap probe so the downloader tests pass cross-platform.

---

Co-authored-by: Codex <codex@openai.com>
- Add a raw ZipInfo helper so the backslash case is not normalized away by platform-specific separator handling.

- Keep the unsafe-member matrix exercising the intended Windows-style rejection path directly.

---

Co-authored-by: Codex <codex@openai.com>
- Add `_safe_zip_member_parts` validation method to reject unsafe raw backslash separators in ZIP member names.
- Skip Windows-specific tests where `zipfile` normalizes backslashes.
- Add new test cases to assert behavior of `_safe_zip_member_parts`.
@Borda Borda merged commit 3b6d910 into develop Jul 9, 2026
19 checks passed
@Borda Borda deleted the fix/downloader branch July 9, 2026 20:30
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.

Security: _extract_zip does not validate zip member paths (Zip Slip)

2 participants