Repository navigation
fix(file-parsers): reject ZIP archives JSZip reads from outside the guarded region - #9773
Conversation
…uarded region The OOXML zip-bomb guard anchored on the tail EOCD and assumed the archive starts at offset 0, and it skipped anything not starting with a ZIP signature. JSZip takes the last EOCD signature anywhere in the buffer and rebases past prepended data, so prefixing an archive with a few bytes disabled every size and inflation check while mammoth and the Word helpers still inflated it in full. The guard now rejects any buffer JSZip would read from a different EOCD or base offset than the one it inspected.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
 [Medium impact] The PR appears safe to merge; no blocking issue was found. SummaryThe PR rejects archives that JSZip would read outside the region checked by the guard.
Reviews (3) · Last reviewed commit: "fix(file-parsers): read the JSZip view w..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…s on the no-op path Treat any saturated EOCD field as ZIP64 in the shared central-directory lookup, as JSZip and the spec do, and share one ZIP64 record reader between the guard and the JSZip-view check. The JSZip-view check now mirrors readEndOfCentral's locator and record search and only flags buffers in which JSZip would actually find central-directory records, so a non-ZIP file with EOCD-like bytes and a saturated field is no longer rejected. readZipCentralDirectoryStats applies the same check so the workspace unzip path fails closed alongside the guard.
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
…mantics JSZip's readInt folds 4- and 8-byte fields through 32-bit shifts, so it sees only the low dword as a signed int, and its signature probe throws on an out-of-range ZIP64 locator pointer rather than falling back. The JSZip-view check read full-width values, so a field with high bits set could make it conclude JSZip would refuse a buffer JSZip actually opens. Read those fields with readInt32LE and mirror the throw conditions. The guard's own walk keeps full-width reads, so any divergence between the two is rejected. The single-use ZIP64 record helper is inlined back into locateCentralDirectory.
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
Summary
assertOoxmlArchiveWithinLimitsanchored on the tail EOCD, assumed the archive starts at offset 0, and silently skipped any buffer not starting with a ZIP signatureArchiveIntegrityError(same path as other unverifiable archives)locateCentralDirectory, and onereadZip64EocdRecordserves both the guard and the JSZip-view checkreadZipCentralDirectoryStatsapplies the same check, so the workspace unzip path fails closed alongside the guardType of Change
Testing
word/document.xmlexpands to 69 MB is rejected plain, passes the guard with an 8-byte prefix, and JSZip/mammoth still read it. With the fix the prefixed copy is rejectedzip-guard.test.ts: prefixed and prefixed + trailing (real JSZip opens them, the guard rejects them; both fail on staging), the ZIP64 trigger (fails on staging and on the first commit), and non-ZIP controls with plain and saturated EOCD-like bytes (the saturated one fails on the first commit)readZipCentralDirectoryStatsbun run testonlib/file-parsers/zip-guard.test.tsandlib/microsoft-word/; earlier run of alllib/file-parsers,lib/microsoft-word,lib/mothership/vfs,lib/uploads,lib/sim-search/livesuites green; biome on changed files. Full type-check/audits in CIChecklist
test-auditauthoring gate)