Skip to content

A zip member that inflates past its declared size is reported unreadable instead of silently truncated - #121

Merged
ww-mw merged 1 commit into
mainfrom
oversize-zip-member-is-unreadable-not-truncated
Oct 16, 2026
Merged

ww-mw merged 1 commit into
mainfrom
oversize-zip-member-is-unreadable-not-truncated

Conversation

@ww-mw

@ww-mw ww-mw commented Oct 16, 2026

Copy link
Copy Markdown
Member

One commit, one item. The commit regenerates dist/ and passes npm run verify.

C-114: a zip member that inflates past its declared size (00c8f97)

The ruling (DECIDED 2026-10-16, group E, theme 1, damaged input): "warn and show what was read; never refuse the file and never crash. C-114 reading: an inflate that would exceed its declared size makes that part unreadable, with a warning; it is never truncated."

The recommendation it adopts: "(a) now: the declared size is free authority and refuses nothing legitimate. Add (b) as a separate decision with a generous default for MAT records, raised as a typed error and never as truncation." Option (a) covers zip members only: "cap each inflate at the member's declared uncompressedSize (native maxOutputLength, or a fixed-size fflate output buffer)."

What was wrong. Take a deflated member whose bytes inflate to more than its central directory entry declares. Both engines returned it cut to the declared size, and nothing said so. Measured at 8e33408:

  • A zip whose a.txt inflates to 100,000 B but declares 1,000 B, beside an intact b.txt, read as { a.txt: 1000, b.txt: 5 } with node:zlib and with fflate (the browser engine).
  • complex_ws.slx with its workspace part declaring 3,000 of its 7,936 bytes opened with 7 workspace rows and no warning. Five of the rows (cfgPTop, pInt16Row, pMixedNF, pRow, pScalar) showed [ ] where the intact file holds 11-12i, [1+2i 3-4i 5+6i] and so on.
  • A member declaring 1 KB that inflates to 200 MB raised peak RSS by 405 MB before anything checked its length.

Root cause. In Inflate.ts the central-directory walk inflated the whole member with native.raw and compared the length only afterwards, so the memory was already spent. A mismatch threw UnsupportedArchive, and the fallback was fflate's unzipSync. That inflates into new u8(declared) with no resize (fflate 0.8.3 inflt) and drops the overflow. With no native engine, unzipSync was the first and only read. A deflate stream of stored blocks overran fflate's fixed buffer with RangeError: offset is out of bounds, which refused the whole archive.

Fix.

  • NativeInflate.raw(deflated, maxOutputLength?). The new parameter is optional, so an injected one-argument engine still type-checks. Detection and the Node subpath (src/node/index.ts) pass { maxOutputLength: Math.max(1, declared) } to inflateRawSync.
  • unzipEntries(bytes, wanted?, unreadable?) is internal; the barrel still exports only setNativeInflate and nativeInflateAvailable. The walk inflates each kept method-8 member under its declared size. ERR_BUFFER_TOO_LARGE, or a result longer than declared (from an engine that ignores the cap), leaves the member out and records it as inflates to more than the N bytes its directory entry declares. There is no fallback for this case, because the fallback truncates. A result shorter than declared still falls back, as before.
  • The fflate read (no native engine, and the fallback) no longer lets unzipSync inflate. unzipSync still frames the archive and raises its own errors (unknown compression type N, invalid zip data). The members are read at fflate's own data offsets, ZIP64 included, and each deflated one is inflated into a buffer one byte past its declared size. A full buffer, or a RangeError from a stored block, means it is over. This differs from the scope's first option, for a reason: running the walk with an fflate engine would not cover the fallback, because the walk refuses ZIP64 and damaged layouts whatever the engine.
  • Callers. parseSlx warns part-unreadable with part set, and the model opens. In parseBinarySldd, an unreadable data/chunk0.xml opens an empty dictionary with one source-unreadable warning, not the "Missing data/chunk0.xml" throw (that throw is for an absent member). Any other member gets part-unreadable. ingest of a .prj warns part-unreadable through a new optional 4th parameter, warnings, on session.addProjectSource, shaped like addDataSource's. This adds to the public Session surface without breaking it. SlddScan and ModelStructureScan read the member as absent.

Tests. 37 new. 33 of them fail before the fix for the stated reason; the other 4 are controls that pass before and after.

  • test/inflateSeam.test.ts, new describe "a member that inflates past its declared size is unreadable, never truncated", 23 cases. Each of 3 engines (node:zlib as detected, fflate, and an injected engine that ignores the cap) gets 6 cases:

    • the member is left out and named, and b.txt is still read. Before: [a.txt, b.txt], with a.txt cut to 1,000 B.
    • it is left out even when the caller passes no list.
    • exactly the declared size reads, and 1 byte over is over. Before: present.
    • a member declaring 0 B that inflates to 1 B is over. Before: present.
    • a stored-block stream over its size is over. Before: RangeError: offset is out of bounds, which refused the archive.
    • an honest archive names nothing (the control).

    Also: the fallback after the walk refuses the archive, and after the native engine throws (before: truncated); a ZIP64 archive, detected and fflate (before: [a.txt, b.txt]); and a spy that sees maxOutputLength [5000, 300] (before: [undefined, undefined]).

  • test/inflateBomb.test.ts, a new file so that its peak RSS is its own: a 200 MB-of-zeros member declaring 1 KB (206 KB deflated), read through loadFromPath (the Node subpath), through detection, and through fflate. The bound is < 16 MB. Before: +404.9 MB on the subpath and on detection, and fflate truncated the member. After: +0.66, +0.05 and +1.25 MB (fflate takes 421 ms). The 4th case, the control, checks that the stream really inflates to 200 MB.

  • test/parseWarnings.test.ts, new describe "a zip member that inflates past its declared size is named unreadable, never read cut short", 5 cases × 2 engines:

    • complex_ws.slx as above opens with its workspace section empty and one part-unreadable naming simulink/modelWorkspace.mxarray. Before: 7 rows and no warning.
    • a binary .sldd whose chunk is over opens empty with source-unreadable, also through ingest. Before: no warning.
    • a binary .sldd whose metadata member is over keeps every entry and warns part-unreadable. Before: no warning.
    • scanSldd reads the over chunk as empty. Before: 14 names from the cut chunk.
    • a .prj whose store document is over warns part-unreadable naming it. Before: the project reader's own XML error for the cut document.

    This warning agrees with C-232's "no warning": A model whose part is missing shows that part's section, empty #114 kept the existing part-level part-unreadable for an unreadable workspace part, and C-114's reading says "with a warning".

  • Two mutations were each caught by a test: the ZIP64 offset read, and the RangeError branch in the fflate read.

Existing tests changed: none. No existing test pinned truncation. "falls back when the native engine returns the wrong number of bytes" returns 3 B, fewer than declared, so it still falls back and stays green.

Speed. fflate path, 81 MB member: median 118 ms, against 127 ms for the old unzipSync. node:zlib: 22.8 ms.

Left out, and why.

  • MAT miCOMPRESSED records declare no size, so they stay unbounded. That is option (b), which the recommendation makes a separate decision.
  • A directory that declares a huge size is still trusted. fflate now allocates declared + 1 where it allocated declared before, so a 4 GB declaration fails to allocate either way.
  • Saving a binary .sldd rezips zipMetadata, so an unreadable metadata member is dropped from the saved package. Before this change it was written back truncated, so it was lost either way. A dictionary whose chunk is unreadable opens empty, with the same save exposure as the existing source-unreadable paths.
  • dex-core's own inflate is out of scope. MATLAB was not needed: this is zip framing, not a MATLAB behaviour.

Gate, at the head (00c8f97)

  • rm -rf dist && npm run build: rc 0, and the tree stays clean (git status --porcelain is empty).
  • npm run verify: rc 0. Typecheck, build, smoke, test (239 files passed, 1 skipped; 7,966 tests passed, 26 skipped, of 7,992), check:pack, check:leak, and check:browser (156 modules reachable from the barrel, none importing a node: builtin).

Implements the maintainer's 2026-10-16 group E ruling.

…cut short

A deflated zip member whose bytes inflate to more than its central directory entry
declares came back cut to the declared size on both engines, with nothing to say so. The
native walk inflated all of it, saw the length disagree and fell back to fflate, whose
unzipSync inflates into a buffer of the declared size and drops the rest. complex_ws.slx
with its workspace part declaring 3,000 of its 7,936 bytes opened with seven workspace
variables, five of them showing [ ] where the file holds 11-12i and the like.

Such a member is now left out of the read and reported, on every engine and path:

- unzipEntries takes an optional list of unreadable members. NativeInflate.raw gains an
  optional maxOutputLength, which detection and the Node subpath pass to inflateRawSync.
  The walk inflates each member under its declared size; ERR_BUFFER_TOO_LARGE, or a result
  longer than declared from an engine that ignores the cap, makes the member unreadable
  instead of a reason to fall back. A result shorter than declared still falls back.
- The fflate read (no native engine, and the fallback) no longer lets unzipSync inflate.
  unzipSync still frames the archive and raises its own diagnostics; the members are read
  at fflate's own offsets, and a deflated one is inflated into a buffer one byte past its
  declared size.
- parseSlx warns part-unreadable for the member. parseBinarySldd opens an unreadable
  data/chunk0.xml as an empty dictionary with one source-unreadable warning, and warns
  part-unreadable for any other member. ingest of a .prj warns part-unreadable, through a
  new optional warnings argument on addProjectSource like addDataSource's. The scans read
  the member as absent.

A member declaring 1 KB that inflates to 200 MB raised the peak RSS by 405 MB; it now
costs 0.05 MB with node:zlib and 1.3 MB with fflate. No existing test changes.

Not covered: MAT miCOMPRESSED records, which declare no size, and a directory that
declares a huge size, which is still trusted.
@ww-mw
ww-mw merged commit f15edbb into main Oct 16, 2026
4 checks passed
@ww-mw
ww-mw deleted the oversize-zip-member-is-unreadable-not-truncated branch October 16, 2026 20:33
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.

1 participant