This is an automated email from the ASF dual-hosted git repository.
pjfanning pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/poi.git
The following commit(s) were added to refs/heads/trunk by this push:
new f1ccdb43d9 Enforce ZipSecureFile limits for encrypted temp ZIP streams
(#1062)
f1ccdb43d9 is described below
commit f1ccdb43d9691a58af0f2f73a48ef6403d7377aa
Author: jmestwa-coder <[email protected]>
AuthorDate: Thu Jun 4 23:42:55 2026 +0530
Enforce ZipSecureFile limits for encrypted temp ZIP streams (#1062)
---
.../poi/openxml4j/util/ZipEntryProcessor.java | 34 +++++++++
.../apache/poi/openxml4j/util/ZipStreamUtil.java | 46 ++++++++++++
.../poifs/crypt/temp/AesZipFileZipEntrySource.java | 38 ++++++----
.../crypt/tests/TestEncryptedTempZipThreshold.java | 83 ++++++++++++++++++++++
.../poi/poifs/crypt/tests/TestSecureTempZip.java | 80 +++++++++++++++++++++
5 files changed, 267 insertions(+), 14 deletions(-)
diff --git
a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipEntryProcessor.java
b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipEntryProcessor.java
new file mode 100644
index 0000000000..627182521b
--- /dev/null
+++
b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipEntryProcessor.java
@@ -0,0 +1,34 @@
+/* ====================================================================
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+==================================================================== */
+
+package org.apache.poi.openxml4j.util;
+
+import java.io.IOException;
+import java.io.InputStream;
+
+import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
+import org.apache.poi.util.Internal;
+
+/**
+ * Minimal processor callback used by {@link ZipStreamUtil#streamEntries}.
+ *
+ * Marked {@link Internal} to indicate this is not a public stable API.
+ */
+@Internal
+public interface ZipEntryProcessor {
+ void process(ZipArchiveEntry entry, InputStream entryStream) throws
IOException;
+}
diff --git
a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipStreamUtil.java
b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipStreamUtil.java
new file mode 100644
index 0000000000..f05e6114b9
--- /dev/null
+++ b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipStreamUtil.java
@@ -0,0 +1,46 @@
+/* ====================================================================
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+==================================================================== */
+
+package org.apache.poi.openxml4j.util;
+
+import java.io.IOException;
+
+import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
+import org.apache.poi.util.Internal;
+
+/**
+ * Small utility to iterate a {@link ZipArchiveThresholdInputStream} and
+ * execute a {@link ZipEntryProcessor} for every entry. Kept intentionally
+ * minimal and annotated {@link Internal} so maintainers can review it as
+ * an internal helper rather than a public API.
+ */
+@Internal
+public final class ZipStreamUtil {
+
+ private ZipStreamUtil() {
+ // utility
+ }
+
+ public static void streamEntries(ZipArchiveThresholdInputStream
zisThreshold, ZipEntryProcessor processor) throws IOException {
+ // Iterate entries using package-private getNextEntry() which preserves
+ // ZipSecureFile enforcement (entry count, size and inflate ratio
checks).
+ ZipArchiveEntry ze;
+ while ((ze = zisThreshold.getNextEntry()) != null) {
+ processor.process(ze, zisThreshold);
+ }
+ }
+}
diff --git
a/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/temp/AesZipFileZipEntrySource.java
b/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/temp/AesZipFileZipEntrySource.java
index e748bc578c..ac55fe843b 100644
---
a/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/temp/AesZipFileZipEntrySource.java
+++
b/poi-ooxml/src/main/java/org/apache/poi/poifs/crypt/temp/AesZipFileZipEntrySource.java
@@ -38,8 +38,9 @@ import org.apache.commons.compress.archivers.zip.ZipFile;
import org.apache.commons.io.output.CloseShieldOutputStream;
import org.apache.logging.log4j.Logger;
import org.apache.poi.logging.PoiLogManager;
-import org.apache.poi.openxml4j.util.ZipEntrySource;
import org.apache.poi.openxml4j.util.ZipArchiveThresholdInputStream;
+import org.apache.poi.openxml4j.util.ZipEntrySource;
+import org.apache.poi.openxml4j.util.ZipStreamUtil;
import org.apache.poi.poifs.crypt.ChainingMode;
import org.apache.poi.poifs.crypt.CipherAlgorithm;
import org.apache.poi.poifs.crypt.CryptoFunctions;
@@ -60,13 +61,15 @@ public final class AesZipFileZipEntrySource implements
ZipEntrySource {
private final File tmpFile;
private final ZipFile zipFile;
- private final Cipher ci;
+ private final SecretKeySpec skeySpec;
+ private final byte[] ivBytes;
private boolean closed;
- private AesZipFileZipEntrySource(File tmpFile, Cipher ci) throws
IOException {
+ private AesZipFileZipEntrySource(File tmpFile, SecretKeySpec skeySpec,
byte[] ivBytes) throws IOException {
this.tmpFile = tmpFile;
this.zipFile = ZipFile.builder().setFile(tmpFile).get();
- this.ci = ci;
+ this.skeySpec = skeySpec;
+ this.ivBytes = ivBytes.clone();
this.closed = false;
}
@@ -87,7 +90,7 @@ public final class AesZipFileZipEntrySource implements
ZipEntrySource {
@Override
public InputStream getInputStream(ZipArchiveEntry entry) throws
IOException {
InputStream is = zipFile.getInputStream(entry);
- return new CipherInputStream(is, ci);
+ return new CipherInputStream(is, getCipher(Cipher.DECRYPT_MODE));
}
@Override
@@ -129,15 +132,16 @@ public final class AesZipFileZipEntrySource implements
ZipEntrySource {
private static void copyToFile(InputStream is, File tmpFile, byte[]
keyBytes, byte[] ivBytes) throws IOException {
SecretKeySpec skeySpec = new SecretKeySpec(keyBytes,
CipherAlgorithm.aes128.jceId);
- Cipher ciEnc = CryptoFunctions.getCipher(skeySpec,
CipherAlgorithm.aes128, ChainingMode.cbc, ivBytes, Cipher.ENCRYPT_MODE,
PADDING);
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) {
+ // Stream entries directly via a minimal internal helper so we
preserve
+ // streaming semantics (no materialization) while still using
+ // ZipArchiveThresholdInputStream's package-local checks.
+ ZipStreamUtil.streamEntries(zis, (ze, entryIn) -> {
// the cipher output stream pads the data, therefore we can't
reuse the ZipEntry with set sizes
// as those will be validated upon close()
ZipArchiveEntry zeNew = new ZipArchiveEntry(ze.getName());
@@ -147,19 +151,25 @@ public final class AesZipFileZipEntrySource implements
ZipEntrySource {
// zeNew.setMethod(ze.getMethod());
zos.putArchiveEntry(zeNew);
+ // create an independent cipher per-entry to avoid sharing
mutable Cipher instances
+ Cipher ciEncEntry = CryptoFunctions.getCipher(skeySpec,
CipherAlgorithm.aes128, ChainingMode.cbc, ivBytes, Cipher.ENCRYPT_MODE,
PADDING);
+
// don't close underlying ZipOutputStream
- try (CipherOutputStream cos = new
CipherOutputStream(CloseShieldOutputStream.wrap(zos), ciEnc)) {
- IOUtils.copy(zis, cos);
+ try (CipherOutputStream cos = new
CipherOutputStream(CloseShieldOutputStream.wrap(zos), ciEncEntry)) {
+ IOUtils.copy(entryIn, cos);
}
zos.closeArchiveEntry();
- }
+ });
}
}
private static AesZipFileZipEntrySource fileToSource(File tmpFile, byte[]
keyBytes, byte[] ivBytes) throws IOException {
SecretKeySpec skeySpec = new SecretKeySpec(keyBytes,
CipherAlgorithm.aes128.jceId);
- Cipher ciDec = CryptoFunctions.getCipher(skeySpec,
CipherAlgorithm.aes128, ChainingMode.cbc, ivBytes, Cipher.DECRYPT_MODE,
PADDING);
- return new AesZipFileZipEntrySource(tmpFile, ciDec);
+ return new AesZipFileZipEntrySource(tmpFile, skeySpec, ivBytes);
+ }
+
+ private Cipher getCipher(int cipherMode) {
+ return CryptoFunctions.getCipher(skeySpec, CipherAlgorithm.aes128,
ChainingMode.cbc, ivBytes, cipherMode, PADDING);
}
-}
\ No newline at end of file
+}
diff --git
a/poi-ooxml/src/test/java/org/apache/poi/poifs/crypt/tests/TestEncryptedTempZipThreshold.java
b/poi-ooxml/src/test/java/org/apache/poi/poifs/crypt/tests/TestEncryptedTempZipThreshold.java
new file mode 100644
index 0000000000..444fa2e29f
--- /dev/null
+++
b/poi-ooxml/src/test/java/org/apache/poi/poifs/crypt/tests/TestEncryptedTempZipThreshold.java
@@ -0,0 +1,83 @@
+/* ====================================================================
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+==================================================================== */
+
+package org.apache.poi.poifs.crypt.tests;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.zip.Deflater;
+
+import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
+import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream;
+import org.apache.poi.openxml4j.util.ZipSecureFile;
+import org.apache.poi.poifs.crypt.temp.AesZipFileZipEntrySource;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.parallel.Isolated;
+
+/**
+ * Regression test proving that encrypted-temp ZIP creation enforces
+ * the {@link ZipSecureFile#MIN_INFLATE_RATIO} check.
+ *
+ * This test constructs a small compressed ZIP entry which expands to a
+ * much larger uncompressed payload (very low inflate ratio). The
+ * encrypted-temp creation must trigger the min-inflate-ratio check and
+ * throw an IOException. The test is deterministic and uses a small
+ * in-memory ZIP payload.
+ */
+@Isolated // changes global ZipSecureFile limits
+public class TestEncryptedTempZipThreshold {
+
+ @Test
+ void minInflateRatioEnforced() throws IOException {
+ final double oldMinInflateRatio = ZipSecureFile.getMinInflateRatio();
+ final long oldGraceEntrySize = ZipSecureFile.getGraceEntrySize();
+ try {
+ // make threshold very strict so small compressed -> large
expanded fails
+ ZipSecureFile.setMinInflateRatio(0.5d);
+ ZipSecureFile.setGraceEntrySize(0);
+
+ // create an in-memory zip with one entry that compresses
extremely well
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ try (ZipArchiveOutputStream zos = new
ZipArchiveOutputStream(baos)) {
+ ZipArchiveEntry ze = new ZipArchiveEntry("large.txt");
+ zos.putArchiveEntry(ze);
+
+ // write a highly compressible payload (repeated 'A')
+ byte[] payload = new byte[200_000];
+ for (int i = 0; i < payload.length; i++) payload[i] = 'A';
+ // use high compression level to make compressed size tiny
+ zos.setLevel(Deflater.BEST_COMPRESSION);
+ zos.write(payload);
+ zos.closeArchiveEntry();
+ zos.finish();
+ }
+
+ byte[] zipBytes = baos.toByteArray();
+ try (InputStream in = new ByteArrayInputStream(zipBytes)) {
+ // createZipEntrySource will attempt to materialize entries
and should fail
+ assertThrows(IOException.class, () ->
AesZipFileZipEntrySource.createZipEntrySource(in));
+ }
+ } finally {
+ ZipSecureFile.setMinInflateRatio(oldMinInflateRatio);
+ ZipSecureFile.setGraceEntrySize(oldGraceEntrySize);
+ }
+ }
+}
diff --git
a/poi-ooxml/src/test/java/org/apache/poi/poifs/crypt/tests/TestSecureTempZip.java
b/poi-ooxml/src/test/java/org/apache/poi/poifs/crypt/tests/TestSecureTempZip.java
index fdebd2793a..df80e13060 100644
---
a/poi-ooxml/src/test/java/org/apache/poi/poifs/crypt/tests/TestSecureTempZip.java
+++
b/poi-ooxml/src/test/java/org/apache/poi/poifs/crypt/tests/TestSecureTempZip.java
@@ -18,6 +18,7 @@
package org.apache.poi.poifs.crypt.tests;
import static org.apache.poi.POITestCase.assertContains;
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -43,15 +44,72 @@ import org.apache.poi.poifs.crypt.Decryptor;
import org.apache.poi.poifs.crypt.EncryptionInfo;
import org.apache.poi.poifs.crypt.temp.AesZipFileZipEntrySource;
import org.apache.poi.poifs.filesystem.POIFSFileSystem;
+import org.apache.poi.util.IOUtils;
import org.apache.poi.xssf.XSSFTestDataSamples;
import org.apache.poi.xssf.extractor.XSSFBEventBasedExcelExtractor;
import org.apache.poi.xssf.extractor.XSSFEventBasedExcelExtractor;
import org.apache.poi.xssf.usermodel.XSSFWorkbook;
import org.apache.xmlbeans.XmlException;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.parallel.Isolated;
+@Isolated // changes global ZipSecureFile limits
class TestSecureTempZip {
+ @Test
+ void encryptedTempZipStreamsUseIndependentCiphers() throws IOException {
+ byte[] payload = new byte[4096];
+ for (int i = 0; i < payload.length; i++) {
+ payload[i] = (byte)(i & 0xff);
+ }
+
+ try (AesZipFileZipEntrySource source =
AesZipFileZipEntrySource.createZipEntrySource(
+ zipInputStream(new ZipPayload("payload.bin", payload)))) {
+ ZipArchiveEntry entry = source.getEntry("payload.bin");
+ try (InputStream first = source.getInputStream(entry);
+ InputStream second = source.getInputStream(entry)) {
+ byte[] firstPrefix = first.readNBytes(127);
+ assertArrayEquals(payload, IOUtils.toByteArray(second));
+
+ ByteArrayOutputStream firstBytes = new ByteArrayOutputStream();
+ firstBytes.write(firstPrefix);
+ firstBytes.write(IOUtils.toByteArray(first));
+ assertArrayEquals(payload, firstBytes.toByteArray());
+ }
+ }
+ }
+
+ @Test
+ void encryptedTempZipCreationAppliesMaxEntrySize() throws IOException {
+ long oldMaxEntrySize = ZipSecureFile.getMaxEntrySize();
+ try {
+ ZipSecureFile.setMaxEntrySize(32);
+ byte[] payload = new byte[128];
+
+ IOException ex = assertThrows(IOException.class, () ->
+
AesZipFileZipEntrySource.createZipEntrySource(zipInputStream(new
ZipPayload("large.bin", payload))));
+ assertTrue(ex.getMessage().contains("Zip bomb detected"));
+ } finally {
+ ZipSecureFile.setMaxEntrySize(oldMaxEntrySize);
+ }
+ }
+
+ @Test
+ void encryptedTempZipCreationAppliesMaxFileCount() throws IOException {
+ long oldMaxFileCount = ZipSecureFile.getMaxFileCount();
+ try {
+ ZipSecureFile.setMaxFileCount(1);
+
+ IOException ex = assertThrows(IOException.class, () ->
+
AesZipFileZipEntrySource.createZipEntrySource(zipInputStream(
+ new ZipPayload("first.bin", new byte[] {1}),
+ new ZipPayload("second.bin", new byte[] {2}))));
+ assertTrue(ex.getMessage().contains("MAX_FILE_COUNT"));
+ } finally {
+ ZipSecureFile.setMaxFileCount(oldMaxFileCount);
+ }
+ }
+
/**
* Test case for #59841 - this is an example on how to use encrypted temp
files,
* which are streamed into POI opposed to having everything in memory
@@ -155,6 +213,18 @@ class TestSecureTempZip {
}
}
+ private static InputStream zipInputStream(ZipPayload... payloads) throws
IOException {
+ ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ try (ZipArchiveOutputStream zos = new ZipArchiveOutputStream(bos)) {
+ for (ZipPayload payload : payloads) {
+ zos.putArchiveEntry(new ZipArchiveEntry(payload.name));
+ zos.write(payload.bytes);
+ zos.closeArchiveEntry();
+ }
+ }
+ return new ByteArrayInputStream(bos.toByteArray());
+ }
+
private static byte[] buildHighlyCompressedZip(String entryName, int
payloadSize) throws IOException {
byte[] payload = new byte[payloadSize];
ByteArrayOutputStream bos = new ByteArrayOutputStream();
@@ -167,4 +237,14 @@ class TestSecureTempZip {
return bos.toByteArray();
}
+ private static final class ZipPayload {
+ private final String name;
+ private final byte[] bytes;
+
+ private ZipPayload(String name, byte[] bytes) {
+ this.name = name;
+ this.bytes = bytes;
+ }
+ }
+
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]