💣 fix: Harden against falsified ZIP metadata in ODT parsing#12320
Merged
Conversation
…sion The ODT decompressed-size guard was checking JSZip's private _data.uncompressedSize fields, which are populated from the ZIP central directory — attacker-controlled metadata. A crafted ODT with falsified uncompressedSize values bypassed the 50MB cap entirely, allowing content.xml decompression to exhaust Node.js heap memory (DoS). Replace JSZip with yauzl for ODT extraction. The new extractOdtContentXml function uses yauzl's streaming API: it lazily iterates ZIP entries, opens a decompression stream for content.xml, and counts real bytes as they arrive from the inflate stream. The stream is destroyed the moment the byte count crosses ODT_MAX_DECOMPRESSED_SIZE, aborting the inflate before the full payload is materialised in memory. - Remove jszip from direct dependencies (still transitive via mammoth) - Add yauzl + @types/yauzl - Update zip-bomb test to verify streaming abort with DEFLATE payload
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens ODT parsing against zip-bomb style DoS by switching from metadata-based decompressed-size checks (attacker-controlled) to streaming decompression with real byte counting and early abort once the 50MB cap is exceeded.
Changes:
- Replaced JSZip-based ODT
content.xmlextraction with yauzl streaming extraction and on-the-fly decompressed byte counting. - Updated/added a Jest test asserting decompression abort behavior when
content.xmlexceeds the limit. - Swapped
jszipforyauzlin@librechat/apidependencies and added@types/yauzl.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/api/src/files/documents/crud.ts | Streams content.xml out of ODTs using yauzl and aborts mid-inflate after exceeding the decompressed-size cap. |
| packages/api/src/files/documents/crud.spec.ts | Updates the ODT zip-bomb test expectation/message to match the new streaming guard behavior. |
| packages/api/package.json | Adds yauzl + typings; removes jszip from runtime deps. |
| package-lock.json | Lockfile updates reflecting dependency changes (yauzl added, jszip removed as direct dep). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
danny-avila
changed the title
- Use a shared `finish()` helper in extractOdtContentXml that calls zipfile.close() on every exit path (success, size cap, missing entry, openReadStream errors, zipfile errors). Without this, any error path leaked one OS file descriptor permanently — uploading many malformed ODTs could exhaust the process FD limit (a distinct DoS vector). - Add jszip to devDependencies so the zip-bomb test has an explicit dependency rather than relying on mammoth's transitive jszip. - Update JSDoc to document that all exit paths close the zipfile.
danny-avila
changed the title
Matches the established pattern for runtime parser libraries in packages/api: mammoth, pdfjs-dist, and xlsx are all peerDependencies (provided by the consuming /api workspace) with devDependencies for testing. yauzl was incorrectly placed in dependencies.
packages/api declares yauzl as a peerDependency; /api is the consuming workspace that must provide it at runtime, matching the pattern used for mammoth, pdfjs-dist, and xlsx.
jcbartle
pushed a commit
to jcbartle/LibreChat
that referenced
this pull request
May 11, 2026
…ila#12320) * security: replace JSZip metadata guard with yauzl streaming decompression The ODT decompressed-size guard was checking JSZip's private _data.uncompressedSize fields, which are populated from the ZIP central directory — attacker-controlled metadata. A crafted ODT with falsified uncompressedSize values bypassed the 50MB cap entirely, allowing content.xml decompression to exhaust Node.js heap memory (DoS). Replace JSZip with yauzl for ODT extraction. The new extractOdtContentXml function uses yauzl's streaming API: it lazily iterates ZIP entries, opens a decompression stream for content.xml, and counts real bytes as they arrive from the inflate stream. The stream is destroyed the moment the byte count crosses ODT_MAX_DECOMPRESSED_SIZE, aborting the inflate before the full payload is materialised in memory. - Remove jszip from direct dependencies (still transitive via mammoth) - Add yauzl + @types/yauzl - Update zip-bomb test to verify streaming abort with DEFLATE payload * fix: close file descriptor leaks and declare jszip test dependency - Use a shared `finish()` helper in extractOdtContentXml that calls zipfile.close() on every exit path (success, size cap, missing entry, openReadStream errors, zipfile errors). Without this, any error path leaked one OS file descriptor permanently — uploading many malformed ODTs could exhaust the process FD limit (a distinct DoS vector). - Add jszip to devDependencies so the zip-bomb test has an explicit dependency rather than relying on mammoth's transitive jszip. - Update JSDoc to document that all exit paths close the zipfile. * fix: move yauzl from dependencies to peerDependencies Matches the established pattern for runtime parser libraries in packages/api: mammoth, pdfjs-dist, and xlsx are all peerDependencies (provided by the consuming /api workspace) with devDependencies for testing. yauzl was incorrectly placed in dependencies. * fix: add yauzl to /api dependencies to satisfy peer dep packages/api declares yauzl as a peerDependency; /api is the consuming workspace that must provide it at runtime, matching the pattern used for mammoth, pdfjs-dist, and xlsx.
ThomasVuNguyen
pushed a commit
to ThomasVuNguyen/LibreChat
that referenced
this pull request
Jul 15, 2026
…ila#12320) * security: replace JSZip metadata guard with yauzl streaming decompression The ODT decompressed-size guard was checking JSZip's private _data.uncompressedSize fields, which are populated from the ZIP central directory — attacker-controlled metadata. A crafted ODT with falsified uncompressedSize values bypassed the 50MB cap entirely, allowing content.xml decompression to exhaust Node.js heap memory (DoS). Replace JSZip with yauzl for ODT extraction. The new extractOdtContentXml function uses yauzl's streaming API: it lazily iterates ZIP entries, opens a decompression stream for content.xml, and counts real bytes as they arrive from the inflate stream. The stream is destroyed the moment the byte count crosses ODT_MAX_DECOMPRESSED_SIZE, aborting the inflate before the full payload is materialised in memory. - Remove jszip from direct dependencies (still transitive via mammoth) - Add yauzl + @types/yauzl - Update zip-bomb test to verify streaming abort with DEFLATE payload * fix: close file descriptor leaks and declare jszip test dependency - Use a shared `finish()` helper in extractOdtContentXml that calls zipfile.close() on every exit path (success, size cap, missing entry, openReadStream errors, zipfile errors). Without this, any error path leaked one OS file descriptor permanently — uploading many malformed ODTs could exhaust the process FD limit (a distinct DoS vector). - Add jszip to devDependencies so the zip-bomb test has an explicit dependency rather than relying on mammoth's transitive jszip. - Update JSDoc to document that all exit paths close the zipfile. * fix: move yauzl from dependencies to peerDependencies Matches the established pattern for runtime parser libraries in packages/api: mammoth, pdfjs-dist, and xlsx are all peerDependencies (provided by the consuming /api workspace) with devDependencies for testing. yauzl was incorrectly placed in dependencies. * fix: add yauzl to /api dependencies to satisfy peer dep packages/api declares yauzl as a peerDependency; /api is the consuming workspace that must provide it at runtime, matching the pattern used for mammoth, pdfjs-dist, and xlsx.
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.
Summary
I replaced the ODT parser's JSZip-based decompression with yauzl's streaming API to close a medium-severity DoS where an authenticated user could bypass the 50MB size guard by falsifying ZIP central directory metadata, causing
content.xmlto expand unboundedly into heap memory.jszipwithyauzlas the production dependency for ODT parsing, removing the previous approach of loading the entire archive into memory viaJSZip.loadAsyncextractOdtContentXml()helper that usesyauzl.openwithlazyEntries: true, streamingcontent.xmlviaopenReadStreamand counting real decompressed bytes as they arrive from the inflate stream — not from attacker-controlled ZIP central directory fieldsreadStream.destroy(err)the moment the real byte count exceedsODT_MAX_DECOMPRESSED_SIZE(50MB), preventing the full allocation from ever completingfinish()helper guarded by asettledflag to ensurezipfile.close()is called exactly once on every exit path — success, size cap breach, missingcontent.xml,openReadStreamerrors, andzipfile-level errors — eliminating file descriptor leaks on all error pathsjsziptodevDependenciesso the zip-bomb test fixture, which constructs a crafted DEFLATE-compressed ODT in-memory, has an explicit declared dependency rather than relying on a transitive one frommammoth@types/yauzltodevDependenciesfor TypeScript declarations, asyauzl@3ships no bundled.d.tsfilesextractOdtContentXmlJSDoc to document both the metadata-bypass protection and the FD lifecycle guaranteeChange Type
Testing
The existing zip-bomb test in
packages/api/src/files/documents/crud.spec.tswas updated to reflect the new guard behavior. It constructs a DEFLATE-compressed ODT in-memory (~51KB on disk, 51MB uncompressed) using JSZip, writes it to a temp file, and asserts the streaming guard rejects with/exceeds the 50MB decompressed limit/before the full content is allocated. Cleanup runs in afinallyblock regardless of outcome.All existing ODT parser tests (
sample.odt,empty.odt,sample-entities.odt) continue to pass against the yauzl-based implementation.Test Configuration:
No environment variables or external services required.
Checklist