Skip to content

zlib: reject ambiguous ZIP archive ends - #65007

Open
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:fix-zip-eocd-ambiguity
Open

zlib: reject ambiguous ZIP archive ends#65007
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:fix-zip-eocd-ambiguity

Conversation

@mcollina

Copy link
Copy Markdown
Member

ZIP archives can contain more than one structurally plausible EOCD record. The previous exact-comment-first search could choose a different central directory depending on trailing padding.

Scan candidates in the same tail window used for file-backed archives and reject multiple plausible interpretations. Invalid EOCD-like comment bytes remain ignored, and padded archives with one interpretation remain supported.

ZIP archives can contain more than one structurally plausible EOCD
record. Selecting based on whether a comment reaches EOF can make the
chosen central directory depend on trailing padding.
Inspect all candidates in the common tail window and reject archives
with multiple plausible interpretations.
Signed-off-by: Matteo Collina <hello@matteocollina.com>
@nodejs-github-botnodejs-github-bot added the needs-ci PRs that need a full CI run. label Aug 4, 2026
@mcollina
mcollina requested review from aduh95 and jasnellAugust 4, 2026 10:40
@pipobscure

Copy link
Copy Markdown
Contributor

I like the fix since it both matches the ecosystem, but still keeps our slightly tighter strictness. There are a few cases where exotic archives would be rejected for being wrong failing closed.

@codecov

codecovBot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.28%. Comparing base (5ba72ae) to head (48a3069).
⚠️ Report is 70 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #65007 +/- ##
==========================================
- Coverage 90.29% 90.28% -0.01% 
==========================================
Files 760 762 +2 Lines 247061 247699 +638 Branches 46586 46718 +132 ==========================================
+ Hits 223076 223642 +566 - Misses 15448 15489 +41 - Partials 8537 8568 +31 
Files with missing linesCoverage Δ
lib/internal/zip/headers.js97.51% <100.00%> (+0.25%)⬆️

... and 70 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@mcollina@pipobscure@panva@nodejs-github-bot