* 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
This commit is contained in:
Dominik Stadler 2015-08-19 10:57:19 +00:00
parent 5706c2ac9b
commit 1e45197f59
3 changed files with 39 additions and 6 deletions

View File

@ -40,6 +40,7 @@ import javax.xml.transform.dom.DOMSource;
import javax.xml.transform.stream.StreamResult; import javax.xml.transform.stream.StreamResult;
import org.apache.poi.openxml4j.opc.internal.ZipHelper; import org.apache.poi.openxml4j.opc.internal.ZipHelper;
import org.apache.poi.openxml4j.util.ZipSecureFile;
import org.apache.poi.util.IOUtils; import org.apache.poi.util.IOUtils;
import org.w3c.dom.Document; import org.w3c.dom.Document;
import org.xml.sax.InputSource; import org.xml.sax.InputSource;
@ -56,6 +57,9 @@ public class OOXMLPrettyPrint {
private final DocumentBuilder documentBuilder; private final DocumentBuilder documentBuilder;
public OOXMLPrettyPrint() throws ParserConfigurationException { 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(); documentBuilder = documentBuilderFactory.newDocumentBuilder();
} }

View File

@ -23,7 +23,6 @@ import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import java.io.PushbackInputStream; import java.io.PushbackInputStream;
import java.lang.reflect.Field; import java.lang.reflect.Field;
import java.nio.charset.Charset;
import java.util.zip.InflaterInputStream; import java.util.zip.InflaterInputStream;
import java.util.zip.ZipEntry; import java.util.zip.ZipEntry;
import java.util.zip.ZipException; import java.util.zip.ZipException;
@ -51,7 +50,8 @@ public class ZipSecureFile extends ZipFile {
/** /**
* Sets the ratio between de- and inflated bytes to detect zipbomb. * 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 * 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 * @param ratio the ratio between de- and inflated bytes to detect zipbomb
*/ */
@ -59,10 +59,24 @@ public class ZipSecureFile extends ZipFile {
MIN_INFLATE_RATIO = 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, * Sets the maximum file size of a single zip entry. It defaults to 4GB,
* i.e. the 32-bit zip format maximum. * 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 * @param maxEntrySize the max. file size of a single zip entry
*/ */
public static void setMaxEntrySize(long maxEntrySize) { public static void setMaxEntrySize(long maxEntrySize) {
@ -72,6 +86,17 @@ public class ZipSecureFile extends ZipFile {
MAX_ENTRY_SIZE = maxEntrySize; 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 { public ZipSecureFile(File file, int mode) throws IOException {
super(file, mode); super(file, mode);
} }
@ -162,9 +187,12 @@ public class ZipSecureFile extends ZipFile {
if (counter < MAX_ENTRY_SIZE) { if (counter < MAX_ENTRY_SIZE) {
if (cis == null) return; if (cis == null) return;
double ratio = (double)cis.counter/(double)counter; 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 { public ZipEntry getNextEntry() throws IOException {

View File

@ -748,12 +748,12 @@ public final class TestPackage {
if(e instanceof InvocationTargetException) { if(e instanceof InvocationTargetException) {
InvocationTargetException t = (InvocationTargetException)e; InvocationTargetException t = (InvocationTargetException)e;
IOException t2 = (IOException)t.getTargetException(); IOException t2 = (IOException)t.getTargetException();
if("Zip bomb detected! Exiting.".equals(t2.getMessage())) { if(t2.getMessage().startsWith("Zip bomb detected!")) {
return; return;
} }
} }
if ("Zip bomb detected! Exiting.".equals(e.getMessage())) { if(e.getMessage().startsWith("Zip bomb detected!")) {
return; 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); 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);
} }
} }