add check for number of files inside zip

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1911588 13f79535-47bb-0310-9956-ffa450edef68

7 files changed, 270 insertions, 7 deletions

diff --git a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/ZipPackage.java b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/ZipPackage.java
index 5c7d798196..cc714e9465 100644
--- a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/ZipPackage.java
+++ b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/ZipPackage.java
@@ -29,6 +29,7 @@ import java.io.OutputStream;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.List;
+import java.util.Locale;
 import java.util.stream.Collectors;
 
 import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
@@ -50,6 +51,7 @@ import org.apache.poi.openxml4j.util.ZipArchiveThresholdInputStream;
 import org.apache.poi.openxml4j.util.ZipEntrySource;
 import org.apache.poi.openxml4j.util.ZipFileZipEntrySource;
 import org.apache.poi.openxml4j.util.ZipInputStreamZipEntrySource;
+import org.apache.poi.openxml4j.util.ZipSecureFile;
 import org.apache.poi.util.IOUtils;
 import org.apache.poi.util.TempFile;
 
@@ -308,8 +310,13 @@ public final class ZipPackage extends OPCPackage {
         // (Need to create relationships before other
         //  parts, otherwise we might create a part before
         //  its relationship exists, and then it won't tie up)
+        final List<? extends ZipArchiveEntry> list = Collections.list(zipEntries);
+        if (list.size() > ZipSecureFile.getMaxFileCount()) {
+            throw new InvalidFormatException(String.format(
+                    Locale.ROOT, ZipSecureFile.MAX_FILE_COUNT_MSG, ZipSecureFile.getMaxFileCount()));
+        }
         final List<EntryTriple> entries =
-                Collections.list(zipEntries).stream()
+                list.stream()
                         .filter(zipArchiveEntry -> !ignoreEntry(zipArchiveEntry))
                         .map(zae -> new EntryTriple(zae, contentTypeManager))
                         .filter(mm -> mm.partName != null)
diff --git a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipArchiveThresholdInputStream.java b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipArchiveThresholdInputStream.java
index 97ed930c37..75e896e78b 100644
--- a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipArchiveThresholdInputStream.java
+++ b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipArchiveThresholdInputStream.java
@@ -17,9 +17,6 @@
 
 package org.apache.poi.openxml4j.util;
 
-import static org.apache.poi.openxml4j.util.ZipSecureFile.MAX_ENTRY_SIZE;
-import static org.apache.poi.openxml4j.util.ZipSecureFile.MIN_INFLATE_RATIO;
-
 import java.io.EOFException;
 import java.io.FilterInputStream;
 import java.io.IOException;
@@ -34,10 +31,10 @@ import org.apache.poi.openxml4j.exceptions.NotOfficeXmlFileException;
 import org.apache.poi.util.IOUtils;
 import org.apache.poi.util.Internal;
 
+import static org.apache.poi.openxml4j.util.ZipSecureFile.*;
+
 @Internal
 public class ZipArchiveThresholdInputStream extends FilterInputStream {
-    // don't alert for expanded sizes smaller than 100k
-    private static final long GRACE_ENTRY_SIZE = 100*1024L;
 
     private static final String MAX_ENTRY_SIZE_MSG =
         "Zip bomb detected! The file would exceed the max size of the expanded data in the zip-file.\n" +
@@ -58,6 +55,7 @@ public class ZipArchiveThresholdInputStream extends FilterInputStream {
      */
     private ZipArchiveEntry entry;
     private boolean guardState = true;
+    private long entryCount;
 
     public ZipArchiveThresholdInputStream(InputStream is) {
         super(is);
@@ -125,7 +123,7 @@ public class ZipArchiveThresholdInputStream extends FilterInputStream {
         final String entryName = entry == null ? "not set" : entry.getName();
 
         // check the file size first, in case we are working on uncompressed streams
-        if(payloadSize > MAX_ENTRY_SIZE) {
+        if (payloadSize > MAX_ENTRY_SIZE) {
             throw new IOException(String.format(Locale.ROOT, MAX_ENTRY_SIZE_MSG, payloadSize, rawSize, MAX_ENTRY_SIZE, entryName));
         }
 
@@ -150,6 +148,11 @@ public class ZipArchiveThresholdInputStream extends FilterInputStream {
 
         try {
             entry = ((ZipArchiveInputStream) in).getNextZipEntry();
+            if (guardState && entry != null) {
+                if (++entryCount > MAX_FILE_COUNT) {
+                    throw new IOException(String.format(Locale.ROOT, MAX_FILE_COUNT_MSG, MAX_FILE_COUNT));
+                }
+            }
             return entry;
         } catch (ZipException ze) {
             if (ze.getMessage().startsWith("Unexpected record signature")) {
diff --git a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipSecureFile.java b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipSecureFile.java
index 8c0bb32899..18509302d1 100644
--- a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipSecureFile.java
+++ b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipSecureFile.java
@@ -37,12 +37,23 @@ public class ZipSecureFile extends ZipFile {
     private static final Logger LOG = LogManager.getLogger(ZipSecureFile.class);
     /* package */ static double MIN_INFLATE_RATIO = 0.01d;
     /* package */ static final long DEFAULT_MAX_ENTRY_SIZE = 0xFFFFFFFFL;
+    /* package */ static final long DEFAULT_MAX_FILE_COUNT = 1000;
+    /* package */ static final long DEFAULT_GRACE_ENTRY_SIZE = 100*1024L;
     /* package */ static long MAX_ENTRY_SIZE = DEFAULT_MAX_ENTRY_SIZE;
+    /* package */ static long MAX_FILE_COUNT = DEFAULT_MAX_FILE_COUNT;
+    /* package */ static long GRACE_ENTRY_SIZE = DEFAULT_GRACE_ENTRY_SIZE;
 
     // The maximum chars of extracted text
     /* package */ static final long DEFAULT_MAX_TEXT_SIZE = 10*1024*1024L;
     private static long MAX_TEXT_SIZE = DEFAULT_MAX_TEXT_SIZE;
 
+    public static final String MAX_FILE_COUNT_MSG =
+            "The file appears to be potentially malicious. This file embeds more internal file entries than expected.\n" +
+                    "This may indicates that the file could pose a security risk.\n" +
+                    "You can adjust this limit via ZipSecureFile.setMaxFileCount() if you need to work with files which are very large.\n" +
+                    "Limits: MAX_FILE_COUNT: %d";
+
+
     private final String fileName;
 
     /**
@@ -69,6 +80,29 @@ public class ZipSecureFile extends ZipFile {
     }
 
     /**
+     * Returns the current maximum file count that is used.
+     *
+     * See setMaxFileCount() for details.
+     *
+     * @return The max accepted file count (i.e. the max number of files we allow inside zip files that we read - including OOXML files like xlsx, docx, pptx, etc.).
+     * @since POI 5.2.4
+     */
+    public static long getMaxFileCount() {
+        return MAX_FILE_COUNT;
+    }
+
+    /**
+     * Sets the maximum file count that we allow inside zip files that we read -
+     * including OOXML files like xlsx, docx, pptx, etc. The default is 1000.
+     *
+     * @param maxFileCount The max accepted file count
+     * @since POI 5.2.4
+     */
+    public static void setMaxFileCount(final long maxFileCount) {
+        MAX_FILE_COUNT = maxFileCount;
+    }
+
+    /**
      * Sets the maximum file size of a single zip entry. It defaults to 4GB,
      * i.e. the 32-bit zip format maximum.
      * 
@@ -99,6 +133,39 @@ public class ZipSecureFile extends ZipFile {
     }
 
     /**
+     * Sets the grace entry size of a single zip entry. It defaults to 100Kb.
+     *
+     * When decompressed data in a zip entry is smaller than this size, the
+     * Minimum Inflation Ratio check is ignored.
+     *
+     * Setting this to a very small value may lead to more files being flagged
+     * as potential Zip Bombs are rejected as a result.
+     *
+     * @param graceEntrySize the grace entry size of a single zip entry
+     * @throws IllegalArgumentException for negative graceEntrySize
+     * @since POI 5.2.4
+     */
+    public static void setGraceEntrySize(long graceEntrySize) {
+        if (graceEntrySize < 0) {
+            throw new IllegalArgumentException("Grace entry size must be greater than or equal to zero");
+        }
+        GRACE_ENTRY_SIZE = graceEntrySize;
+    }
+
+    /**
+     * Returns the current threshold for decompressed data in zip entries that are regarded as too small
+     * to worry about from a Zip Bomb perspective (default is 100Kb).
+     *
+     * See setGraceEntrySize() for details.
+     *
+     * @return The current grace entry size
+     * @since POI 5.2.4
+     */
+    public static long getGraceEntrySize() {
+        return GRACE_ENTRY_SIZE;
+    }
+
+    /**
      * Sets the maximum number of characters of text that are
      * extracted before an exception is thrown during extracting
      * text from documents.
diff --git a/poi-ooxml/src/test/java/org/apache/poi/openxml4j/TestOPCPackageFileLimit.java b/poi-ooxml/src/test/java/org/apache/poi/openxml4j/TestOPCPackageFileLimit.java
new file mode 100644
index 0000000000..017de0a10e
--- /dev/null
+++ b/poi-ooxml/src/test/java/org/apache/poi/openxml4j/TestOPCPackageFileLimit.java
@@ -0,0 +1,64 @@
+/* ====================================================================
+   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;
+
+import org.apache.poi.hssf.HSSFTestDataSamples;
+import org.apache.poi.openxml4j.exceptions.InvalidFormatException;
+import org.apache.poi.openxml4j.opc.OPCPackage;
+import org.apache.poi.openxml4j.util.ZipSecureFile;
+import org.junit.jupiter.api.Test;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
+
+class TestOPCPackageFileLimit {
+    @Test
+    void testWithReducedFileLimit() throws InvalidFormatException {
+        final long defaultLimit = ZipSecureFile.getMaxFileCount();
+        ZipSecureFile.setMaxFileCount(5);
+        try (InputStream is = HSSFTestDataSamples.openSampleFileStream("HeaderFooterComplexFormats.xlsx")) {
+            OPCPackage opcPackage = OPCPackage.open(is);
+            fail("expected IOException");
+        } catch (IOException e) {
+            assertTrue(e.getMessage().contains("ZipSecureFile.setMaxFileCount()"),
+                    "unexpected exception message: " + e.getMessage());
+        } finally {
+            ZipSecureFile.setMaxFileCount(defaultLimit);
+        }
+    }
+
+    @Test
+    void testFileWithReducedFileLimit() {
+        final File file = HSSFTestDataSamples.getSampleFile("HeaderFooterComplexFormats.xlsx");
+        final long defaultLimit = ZipSecureFile.getMaxFileCount();
+        ZipSecureFile.setMaxFileCount(5);
+        try {
+            OPCPackage opcPackage = OPCPackage.open(file);
+            fail("expected InvalidFormatException");
+        } catch (InvalidFormatException e) {
+            assertTrue(e.getMessage().contains("ZipSecureFile.setMaxFileCount()"),
+                    "unexpected exception message: " + e.getMessage());
+        } finally {
+            ZipSecureFile.setMaxFileCount(defaultLimit);
+        }
+    }
+}
diff --git a/poi-ooxml/src/test/java/org/apache/poi/openxml4j/util/TestZipSecureFile.java b/poi-ooxml/src/test/java/org/apache/poi/openxml4j/util/TestZipSecureFile.java
index e639e2fee8..064b0825f3 100644
--- a/poi-ooxml/src/test/java/org/apache/poi/openxml4j/util/TestZipSecureFile.java
+++ b/poi-ooxml/src/test/java/org/apache/poi/openxml4j/util/TestZipSecureFile.java
@@ -79,4 +79,25 @@ class TestZipSecureFile {
             ZipSecureFile.setMaxTextSize(ZipSecureFile.DEFAULT_MAX_TEXT_SIZE);
         }
     }
+
+    @Test
+    void testSettingGraceEntrySize() {
+        long approx8G = ZipSecureFile.MAX_ENTRY_SIZE * 2;
+        try {
+            ZipSecureFile.setGraceEntrySize(approx8G);
+            assertEquals(approx8G, ZipSecureFile.getGraceEntrySize());
+        } finally {
+            ZipSecureFile.setGraceEntrySize(ZipSecureFile.DEFAULT_GRACE_ENTRY_SIZE);
+        }
+    }
+
+    @Test
+    void testSettingMaxFileCount() {
+        try {
+            ZipSecureFile.setMaxFileCount(123456789);
+            assertEquals(123456789, ZipSecureFile.getMaxFileCount());
+        } finally {
+            ZipSecureFile.setMaxFileCount(ZipSecureFile.DEFAULT_MAX_FILE_COUNT);
+        }
+    }
 }
diff --git a/poi-ooxml/src/test/java/org/apache/poi/xssf/TestXSSFFileChecks.java b/poi-ooxml/src/test/java/org/apache/poi/xssf/TestXSSFFileChecks.java
new file mode 100644
index 0000000000..2fc978bd35
--- /dev/null
+++ b/poi-ooxml/src/test/java/org/apache/poi/xssf/TestXSSFFileChecks.java
@@ -0,0 +1,100 @@
+/* ====================================================================
+   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.xssf;
+
+import org.apache.poi.hssf.HSSFTestDataSamples;
+import org.apache.poi.openxml4j.exceptions.InvalidFormatException;
+import org.apache.poi.openxml4j.util.ZipSecureFile;
+import org.apache.poi.xssf.usermodel.XSSFWorkbook;
+import org.junit.jupiter.api.Test;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
+
+class TestXSSFFileChecks {
+    @Test
+    void testWithReducedFileLimit() {
+        final long defaultLimit = ZipSecureFile.getMaxFileCount();
+        ZipSecureFile.setMaxFileCount(5);
+        try (InputStream is = HSSFTestDataSamples.openSampleFileStream("HeaderFooterComplexFormats.xlsx")) {
+            XSSFWorkbook xssfWorkbook = new XSSFWorkbook(is);
+            fail("expected IOException");
+        } catch (IOException e) {
+            assertTrue(e.getMessage().contains("ZipSecureFile.setMaxFileCount()"),
+                    "unexpected exception message: " + e.getMessage());
+        } finally {
+            ZipSecureFile.setMaxFileCount(defaultLimit);
+        }
+    }
+
+    @Test
+    void testFileWithReducedFileLimit() throws IOException {
+        final File file = HSSFTestDataSamples.getSampleFile("HeaderFooterComplexFormats.xlsx");
+        final long defaultLimit = ZipSecureFile.getMaxFileCount();
+        ZipSecureFile.setMaxFileCount(5);
+        try {
+            XSSFWorkbook xssfWorkbook = new XSSFWorkbook(file);
+            fail("expected InvalidFormatException");
+        } catch (InvalidFormatException e) {
+            assertTrue(e.getMessage().contains("ZipSecureFile.setMaxFileCount()"),
+                    "unexpected exception message: " + e.getMessage());
+        } finally {
+            ZipSecureFile.setMaxFileCount(defaultLimit);
+        }
+    }
+
+    @Test
+    void testWithGraceEntrySize() throws IOException {
+        final double defaultInflateRatio = ZipSecureFile.getMinInflateRatio();
+        // setting MinInflateRatio but the default GraceEntrySize will mean this is ignored
+        // this exception will not happen with the default GraceEntrySize
+        ZipSecureFile.setMinInflateRatio(0.50);
+        try (InputStream is = HSSFTestDataSamples.openSampleFileStream("HeaderFooterComplexFormats.xlsx")) {
+            XSSFWorkbook xssfWorkbook = new XSSFWorkbook(is);
+            assertNotNull(xssfWorkbook);
+        } finally {
+            ZipSecureFile.setMinInflateRatio(defaultInflateRatio);
+        }
+    }
+
+    @Test
+    void testWithReducedGraceEntrySize() {
+        final long defaultGraceSize = ZipSecureFile.getGraceEntrySize();
+        final double defaultInflateRatio = ZipSecureFile.getMinInflateRatio();
+        ZipSecureFile.setGraceEntrySize(0);
+        // setting MinInflateRatio to cause an exception
+        // this exception will not happen with the default GraceEntrySize
+        ZipSecureFile.setMinInflateRatio(0.50);
+        try (InputStream is = HSSFTestDataSamples.openSampleFileStream("HeaderFooterComplexFormats.xlsx")) {
+            XSSFWorkbook xssfWorkbook = new XSSFWorkbook(is);
+            fail("expected IOException");
+        } catch (IOException e) {
+            assertTrue(e.getMessage().contains("ZipSecureFile.setMinInflateRatio()"),
+                    "unexpected exception message: " + e.getMessage());
+        } finally {
+            ZipSecureFile.setMinInflateRatio(defaultInflateRatio);
+            ZipSecureFile.setGraceEntrySize(defaultGraceSize);
+        }
+    }
+
+}
diff --git a/poi-ooxml/src/test/java9/module-info.java b/poi-ooxml/src/test/java9/module-info.java
index 7ed78a0e75..d385eb2142 100644
--- a/poi-ooxml/src/test/java9/module-info.java
+++ b/poi-ooxml/src/test/java9/module-info.java
@@ -81,6 +81,7 @@ module org.apache.poi.ooxml {
     exports org.apache.poi.poifs.crypt.temp;
 
     opens org.apache.poi.openxml4j.opc to org.apache.poi.poi, org.junit.platform.commons;
+    opens org.apache.poi.openxml4j to org.apache.poi.ooxml, org.junit.platform.commons;
 
 
     /* optional dependencies for xml signatures - you need to add a require entry your module-info