Skip to content

Apply zip bomb checks to encrypted temp ZIP processing - #1057

Merged
pjfanning merged 1 commit into
apache:trunkfrom
jmestwa-coder:zip-bomb-check-encrypted-temp-zip
May 11, 2026
Merged

pjfanning merged 1 commit into
apache:trunkfrom
jmestwa-coder:zip-bomb-check-encrypted-temp-zip

Conversation

@jmestwa-coder

Copy link
Copy Markdown
Contributor

Summary

Apply existing zip-bomb protections to the encrypted temp ZIP processing path used by AesZipFileZipEntrySource.

Previously, this flow re-streamed attacker-controlled ZIP entries through ZipArchiveInputStream without ZipArchiveThresholdInputStream enforcement, allowing highly compressed entries to bypass configured inflate-ratio checks during temporary encrypted ZIP creation.

This change aligns the encrypted processing path with the existing ZIP security protections already used elsewhere in POI.

Changes

  • wrap encrypted temp ZIP input streams with ZipArchiveThresholdInputStream
  • enforce configured inflate-ratio and entry-threshold checks during temp ZIP creation
  • expose getNextEntry() on ZipArchiveThresholdInputStream for internal stream-based processing usage
  • add regression coverage for highly compressed ZIP input

Regression Test

Added rejectsZipBombInput() to verify:

  • before this patch: highly compressed input was processed successfully
  • after this patch: inflate-ratio enforcement correctly rejects the input with a zip-bomb exception

The test restores modified ZipSecureFile settings in a finally block to avoid leaking state across tests.

}

ZipArchiveEntry getNextEntry() throws IOException {
public ZipArchiveEntry getNextEntry() throws IOException {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert this please

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be public if it is needed to be public

@jmestwa-coder
jmestwa-coder force-pushed the zip-bomb-check-encrypted-temp-zip branch from 6b3e938 to db43a62 Compare May 11, 2026 14:08
@jmestwa-coder

Copy link
Copy Markdown
Contributor Author

@pjfanning Thanks for the review. updated the patch with the visibility revert and formatting fix.


ZipArchiveEntry ze;
while ((ze = zis.getNextEntry()) != null) {
while ((ze = zipStream.getNextEntry()) != null) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't seem right - shouldn't you call zis.getNextEntry? otherwise, what is the point of creating a ZipArchiveThresholdInputStream?

@jmestwa-coder
jmestwa-coder force-pushed the zip-bomb-check-encrypted-temp-zip branch from db43a62 to d1760d1 Compare May 11, 2026 19:06
@jmestwa-coder

Copy link
Copy Markdown
Contributor Author

@pjfanning updated the patch to restore traversal through zis.getNextEntry() and simplified the implementation accordingly. Please let me know if you'd prefer any further adjustments.

@pjfanning pjfanning left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@pjfanning
pjfanning merged commit 7607d1b into apache:trunk May 11, 2026
1 check passed
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.

2 participants