From: Dominik Stadler Date: Wed, 19 Aug 2015 10:57:19 +0000 (+0000) Subject: * Adjust reported text when a Zip-Bomb is detected to allow to quickly see why it... X-Git-Tag: REL_3_13_FINAL~101 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=255ace6918dbdf7195e4e2a0db08284ed0131993;p=poi.git * Adjust reported text when a Zip-Bomb is detected to allow to quickly see why it happened * Fail only with inflation ratio lower than the min, not equals, to behave as documented * Add getters to be able to temporarily adjust the limits for unit tests * Allow lower inflation ratio in OOXMLPrettyPrint as this is a dev-only tool git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1696556 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/src/ooxml/java/org/apache/poi/dev/OOXMLPrettyPrint.java b/src/ooxml/java/org/apache/poi/dev/OOXMLPrettyPrint.java index a05af6b4d8..1a7a9d6c58 100644 --- a/src/ooxml/java/org/apache/poi/dev/OOXMLPrettyPrint.java +++ b/src/ooxml/java/org/apache/poi/dev/OOXMLPrettyPrint.java @@ -40,6 +40,7 @@ import javax.xml.transform.dom.DOMSource; import javax.xml.transform.stream.StreamResult; import org.apache.poi.openxml4j.opc.internal.ZipHelper; +import org.apache.poi.openxml4j.util.ZipSecureFile; import org.apache.poi.util.IOUtils; import org.w3c.dom.Document; import org.xml.sax.InputSource; @@ -56,6 +57,9 @@ public class OOXMLPrettyPrint { private final DocumentBuilder documentBuilder; public OOXMLPrettyPrint() throws ParserConfigurationException { + // allow files with much lower inflation rate here as there is no risk of Zip Bomb attacks in this developer tool + ZipSecureFile.setMinInflateRatio(0.00001); + documentBuilder = documentBuilderFactory.newDocumentBuilder(); } diff --git a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java index 60b31ee278..5423ea4d47 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java @@ -23,7 +23,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.PushbackInputStream; import java.lang.reflect.Field; -import java.nio.charset.Charset; import java.util.zip.InflaterInputStream; import java.util.zip.ZipEntry; import java.util.zip.ZipException; @@ -51,17 +50,32 @@ public class ZipSecureFile extends ZipFile { /** * Sets the ratio between de- and inflated bytes to detect zipbomb. * It defaults to 1% (= 0.01d), i.e. when the compression is better than - * 1% for any given read package part, the parsing will fail + * 1% for any given read package part, the parsing will fail indicating a + * Zip-Bomb. * * @param ratio the ratio between de- and inflated bytes to detect zipbomb */ public static void setMinInflateRatio(double ratio) { MIN_INFLATE_RATIO = ratio; } + + /** + * Returns the current minimum compression rate that is used. + * + * See setMinInflateRatio() for details. + * + * @return The min accepted compression-ratio. + */ + public static double getMinInflateRatio() { + return MIN_INFLATE_RATIO; + } /** * Sets the maximum file size of a single zip entry. It defaults to 4GB, * i.e. the 32-bit zip format maximum. + * + * This can be used to limit memory consumption and protect against + * security vulnerabilities when documents are provided by users. * * @param maxEntrySize the max. file size of a single zip entry */ @@ -72,6 +86,17 @@ public class ZipSecureFile extends ZipFile { MAX_ENTRY_SIZE = maxEntrySize; } + /** + * Returns the current maximum allowed uncompressed file size. + * + * See setMaxEntrySize() for details. + * + * @return The max accepted uncompressed file size. + */ + public static long getMaxEntrySize() { + return MAX_ENTRY_SIZE; + } + public ZipSecureFile(File file, int mode) throws IOException { super(file, mode); } @@ -162,9 +187,12 @@ public class ZipSecureFile extends ZipFile { if (counter < MAX_ENTRY_SIZE) { if (cis == null) return; double ratio = (double)cis.counter/(double)counter; - if (ratio > MIN_INFLATE_RATIO) return; + if (ratio >= MIN_INFLATE_RATIO) return; } - throw new IOException("Zip bomb detected! Exiting."); + throw new IOException("Zip bomb detected! The file would exceed certain limits which usually indicate that the file is used to inflate memory usage and thus could pose a security risk. " + + "You can adjust these limits via setMinInflateRatio() and setMaxEntrySize() if you need to work with files which exceed these limits. " + + "Counter: " + counter + ", cis.counter: " + (cis == null ? 0 : cis.counter) + ", ratio: " + (cis == null ? 0 : ((double)cis.counter)/counter) + + "Limits: MIN_INFLATE_RATIO: " + MIN_INFLATE_RATIO + ", MAX_ENTRY_SIZE: " + MAX_ENTRY_SIZE); } public ZipEntry getNextEntry() throws IOException { diff --git a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java index a6955958fb..6e3b89f53e 100644 --- a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java +++ b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java @@ -748,12 +748,12 @@ public final class TestPackage { if(e instanceof InvocationTargetException) { InvocationTargetException t = (InvocationTargetException)e; IOException t2 = (IOException)t.getTargetException(); - if("Zip bomb detected! Exiting.".equals(t2.getMessage())) { + if(t2.getMessage().startsWith("Zip bomb detected!")) { return; } } - if ("Zip bomb detected! Exiting.".equals(e.getMessage())) { + if(e.getMessage().startsWith("Zip bomb detected!")) { return; } @@ -766,3 +766,4 @@ public final class TestPackage { throw new IllegalStateException("Expected to catch an Exception because of a detected Zip Bomb, but did not find the related error message in the exception", e); } } +