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 00000000000..627182521bc --- /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 00000000000..7e80fdfc57a --- /dev/null +++ b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipStreamUtil.java @@ -0,0 +1,51 @@ +/* ==================================================================== + 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.commons.compress.archivers.zip.ZipArchiveInputStream; +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 { + if (!(zisThreshold instanceof ZipArchiveThresholdInputStream)) { + throw new IllegalArgumentException("Expected ZipArchiveThresholdInputStream"); + } + + // 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 e748bc578ce..9248bcc4eec 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,6 +38,9 @@ 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.ZipArchiveThresholdInputStream; +import org.apache.poi.openxml4j.util.ZipEntryProcessor; +import org.apache.poi.openxml4j.util.ZipStreamUtil; import org.apache.poi.openxml4j.util.ZipEntrySource; import org.apache.poi.openxml4j.util.ZipArchiveThresholdInputStream; import org.apache.poi.poifs.crypt.ChainingMode; @@ -60,13 +63,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 +92,7 @@ public ZipArchiveEntry getEntry(String path) { @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 @@ -136,8 +141,10 @@ private static void copyToFile(InputStream is, File tmpFile, byte[] keyBytes, by 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 +154,25 @@ private static void copyToFile(InputStream is, File tmpFile, byte[] keyBytes, by // 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 00000000000..35367bb3020 --- /dev/null +++ b/poi-ooxml/src/test/java/org/apache/poi/poifs/crypt/tests/TestEncryptedTempZipThreshold.java @@ -0,0 +1,74 @@ +/* ==================================================================== + 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; + +/** + * 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. + */ +public class TestEncryptedTempZipThreshold { + + @Test + void minInflateRatioEnforced() throws IOException { + // 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)); + } + } +} 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 90e4b018fea..197c9ae9eae 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 @@ -17,6 +17,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more package org.apache.poi.poifs.crypt.tests; +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; @@ -42,6 +43,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more 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; @@ -51,6 +53,60 @@ Licensed to the Apache Software Foundation (ASF) under one or more 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 +211,18 @@ void rejectsZipBombInput() throws IOException { } } + 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 +235,13 @@ private static byte[] buildHighlyCompressedZip(String entryName, int payloadSize 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; + } + }