Repository navigation
A zip member that inflates past its declared size is reported unreadable instead of silently truncated - #121
Merged
Conversation
…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.
This was referenced Oct 16, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
One commit, one item. The commit regenerates
dist/and passesnpm 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.txtinflates to 100,000 B but declares 1,000 B, beside an intactb.txt, read as{ a.txt: 1000, b.txt: 5 }with node:zlib and with fflate (the browser engine).complex_ws.slxwith 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 holds11-12i,[1+2i 3-4i 5+6i]and so on.Root cause. In
Inflate.tsthe central-directory walk inflated the whole member withnative.rawand compared the length only afterwards, so the memory was already spent. A mismatch threwUnsupportedArchive, and the fallback was fflate'sunzipSync. That inflates intonew u8(declared)with no resize (fflate 0.8.3inflt) and drops the overflow. With no native engine,unzipSyncwas the first and only read. A deflate stream of stored blocks overran fflate's fixed buffer withRangeError: 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) }toinflateRawSync.unzipEntries(bytes, wanted?, unreadable?)is internal; the barrel still exports onlysetNativeInflateandnativeInflateAvailable. 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 asinflates 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.unzipSyncinflate.unzipSyncstill 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 aRangeErrorfrom 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.parseSlxwarnspart-unreadablewithpartset, and the model opens. InparseBinarySldd, an unreadabledata/chunk0.xmlopens an empty dictionary with onesource-unreadablewarning, not the "Missing data/chunk0.xml" throw (that throw is for an absent member). Any other member getspart-unreadable.ingestof a.prjwarnspart-unreadablethrough a new optional 4th parameter,warnings, onsession.addProjectSource, shaped likeaddDataSource's. This adds to the public Session surface without breaking it.SlddScanandModelStructureScanread 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:b.txtis still read. Before:[a.txt, b.txt], witha.txtcut to 1,000 B.RangeError: offset is out of bounds, which refused the archive.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 seesmaxOutputLength[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 throughloadFromPath(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.slxas above opens with its workspace section empty and onepart-unreadablenamingsimulink/modelWorkspace.mxarray. Before: 7 rows and no warning..slddwhose chunk is over opens empty withsource-unreadable, also throughingest. Before: no warning..slddwhose metadata member is over keeps every entry and warnspart-unreadable. Before: no warning.scanSlddreads the over chunk as empty. Before: 14 names from the cut chunk..prjwhose store document is over warnspart-unreadablenaming 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-unreadablefor 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
RangeErrorbranch 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.
miCOMPRESSEDrecords declare no size, so they stay unbounded. That is option (b), which the recommendation makes a separate decision.declared + 1where it allocateddeclaredbefore, so a 4 GB declaration fails to allocate either way..slddrezipszipMetadata, 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 existingsource-unreadablepaths.Gate, at the head (00c8f97)
rm -rf dist && npm run build: rc 0, and the tree stays clean (git status --porcelainis 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 anode:builtin).Implements the maintainer's 2026-10-16 group E ruling.