You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@poi.apache.org by "jmestwa-coder (via GitHub)" <gi...@apache.org> on 2026/05/06 17:34:01 UTC

[PR] Apply zip bomb checks to encrypted temp ZIP processing [poi]

jmestwa-coder opened a new pull request, #1057:
URL: https://github.com/apache/poi/pull/1057

   ## 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.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


Re: [PR] Apply zip bomb checks to encrypted temp ZIP processing [poi]

Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #1057:
URL: https://github.com/apache/poi/pull/1057#discussion_r3213166988


##########
poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipArchiveThresholdInputStream.java:
##########
@@ -148,7 +148,7 @@ private void checkThreshold() throws IOException {
         throw new IOException(String.format(Locale.ROOT, MIN_INFLATE_RATIO_MSG, payloadSize, rawSize, ratio, MIN_INFLATE_RATIO, entryName));
     }
 
-    ZipArchiveEntry getNextEntry() throws IOException {
+    public ZipArchiveEntry getNextEntry() throws IOException {

Review Comment:
   revert this please



##########
poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/temp/AesZipFileZipEntrySource.java:
##########
@@ -130,9 +131,10 @@ private static void copyToFile(InputStream is, File tmpFile, byte[] keyBytes, by
         SecretKeySpec skeySpec = new SecretKeySpec(keyBytes, CipherAlgorithm.aes128.jceId);
         Cipher ciEnc = CryptoFunctions.getCipher(skeySpec, CipherAlgorithm.aes128, ChainingMode.cbc, ivBytes, Cipher.ENCRYPT_MODE, PADDING);
 
-        try (ZipArchiveInputStream zis = new ZipArchiveInputStream(is);
-             OutputStream fos = Files.newOutputStream(tmpFile.toPath());
-             ZipArchiveOutputStream zos = new ZipArchiveOutputStream(fos)) {
+                try (ZipArchiveInputStream zipStream = new ZipArchiveInputStream(is);

Review Comment:
   proper formatting please - no tabs



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


Re: [PR] Apply zip bomb checks to encrypted temp ZIP processing [poi]

Posted by "jmestwa-coder (via GitHub)" <gi...@apache.org>.
jmestwa-coder commented on PR #1057:
URL: https://github.com/apache/poi/pull/1057#issuecomment-4421488582

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


Re: [PR] Apply zip bomb checks to encrypted temp ZIP processing [poi]

Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning merged PR #1057:
URL: https://github.com/apache/poi/pull/1057


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


Re: [PR] Apply zip bomb checks to encrypted temp ZIP processing [poi]

Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #1057:
URL: https://github.com/apache/poi/pull/1057#discussion_r3221219711


##########
poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipArchiveThresholdInputStream.java:
##########
@@ -148,7 +148,7 @@ private void checkThreshold() throws IOException {
         throw new IOException(String.format(Locale.ROOT, MIN_INFLATE_RATIO_MSG, payloadSize, rawSize, ratio, MIN_INFLATE_RATIO, entryName));
     }
 
-    ZipArchiveEntry getNextEntry() throws IOException {
+    public ZipArchiveEntry getNextEntry() throws IOException {

Review Comment:
   this can be public if it is needed to be public



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


Re: [PR] Apply zip bomb checks to encrypted temp ZIP processing [poi]

Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #1057:
URL: https://github.com/apache/poi/pull/1057#discussion_r3220032570


##########
poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/temp/AesZipFileZipEntrySource.java:
##########
@@ -130,12 +131,13 @@ private static void copyToFile(InputStream is, File tmpFile, byte[] keyBytes, by
         SecretKeySpec skeySpec = new SecretKeySpec(keyBytes, CipherAlgorithm.aes128.jceId);
         Cipher ciEnc = CryptoFunctions.getCipher(skeySpec, CipherAlgorithm.aes128, ChainingMode.cbc, ivBytes, Cipher.ENCRYPT_MODE, PADDING);
 
-        try (ZipArchiveInputStream zis = new ZipArchiveInputStream(is);
+        try (ZipArchiveInputStream zipStream = new ZipArchiveInputStream(is);
+             ZipArchiveThresholdInputStream zis = new ZipArchiveThresholdInputStream(zipStream);
              OutputStream fos = Files.newOutputStream(tmpFile.toPath());
              ZipArchiveOutputStream zos = new ZipArchiveOutputStream(fos)) {
 
             ZipArchiveEntry ze;
-            while ((ze = zis.getNextEntry()) != null) {
+            while ((ze = zipStream.getNextEntry()) != null) {

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



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


Re: [PR] Apply zip bomb checks to encrypted temp ZIP processing [poi]

Posted by "jmestwa-coder (via GitHub)" <gi...@apache.org>.
jmestwa-coder commented on PR #1057:
URL: https://github.com/apache/poi/pull/1057#issuecomment-4423978420

   @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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org