From 5a42a0cd0538f37d527b096bad2dd08095f27d1c Mon Sep 17 00:00:00 2001 From: Andreas Beeker Date: Tue, 23 Jun 2015 23:39:07 +0000 Subject: [PATCH] Bug 56865 - Limit number of bytes (by counting them) while opening office docs Bug 50090 - 'zip' bomb prevention git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1687148 13f79535-47bb-0310-9956-ffa450edef68 --- .../xssf/usermodel/examples/BigGridDemo.java | 3 +- .../org/apache/poi/dev/OOXMLPrettyPrint.java | 3 +- .../apache/poi/openxml4j/opc/ZipPackage.java | 8 +- .../poi/openxml4j/opc/internal/ZipHelper.java | 5 +- .../util/ZipInputStreamZipEntrySource.java | 6 +- .../poi/openxml4j/util/ZipSecureFile.java | 227 ++++++++++++++++++ .../org/apache/poi/xssf/dev/XSSFDump.java | 3 +- .../poi/xssf/streaming/SXSSFWorkbook.java | 3 +- .../poi/openxml4j/opc/AllOpenXML4JTests.java | 38 ++- .../apache/poi/openxml4j/opc/TestPackage.java | 180 ++++++++++++-- .../ExcelFileFormatDocFunctionExtractor.java | 1 + 11 files changed, 427 insertions(+), 50 deletions(-) create mode 100644 src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java diff --git a/src/examples/src/org/apache/poi/xssf/usermodel/examples/BigGridDemo.java b/src/examples/src/org/apache/poi/xssf/usermodel/examples/BigGridDemo.java index 00c5342746..2dbfccec41 100644 --- a/src/examples/src/org/apache/poi/xssf/usermodel/examples/BigGridDemo.java +++ b/src/examples/src/org/apache/poi/xssf/usermodel/examples/BigGridDemo.java @@ -23,6 +23,7 @@ import java.util.zip.ZipEntry; import java.util.zip.ZipFile; import java.util.zip.ZipOutputStream; +import org.apache.poi.openxml4j.opc.internal.ZipHelper; import org.apache.poi.ss.usermodel.DateUtil; import org.apache.poi.ss.usermodel.IndexedColors; import org.apache.poi.ss.util.CellReference; @@ -165,7 +166,7 @@ public class BigGridDemo { * @param out the stream to write the result to */ private static void substitute(File zipfile, File tmpfile, String entry, OutputStream out) throws IOException { - ZipFile zip = new ZipFile(zipfile); + ZipFile zip = ZipHelper.openZipFile(zipfile); ZipOutputStream zos = new ZipOutputStream(out); diff --git a/src/ooxml/java/org/apache/poi/dev/OOXMLPrettyPrint.java b/src/ooxml/java/org/apache/poi/dev/OOXMLPrettyPrint.java index 48341d0c9a..97e16f7589 100644 --- a/src/ooxml/java/org/apache/poi/dev/OOXMLPrettyPrint.java +++ b/src/ooxml/java/org/apache/poi/dev/OOXMLPrettyPrint.java @@ -39,6 +39,7 @@ import javax.xml.transform.TransformerFactory; import javax.xml.transform.dom.DOMSource; import javax.xml.transform.stream.StreamResult; +import org.apache.poi.openxml4j.opc.internal.ZipHelper; import org.apache.poi.util.IOUtils; import org.w3c.dom.Document; import org.xml.sax.InputSource; @@ -82,7 +83,7 @@ public class OOXMLPrettyPrint { IOException, TransformerException, ParserConfigurationException { System.out.println("Reading zip-file " + file + " and writing pretty-printed XML to " + outFile); - ZipFile zipFile = new ZipFile(file); + ZipFile zipFile = ZipHelper.openZipFile(file); try { ZipOutputStream out = new ZipOutputStream(new BufferedOutputStream(new FileOutputStream(outFile))); try { diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java index 888f501302..8937f8e53f 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java @@ -41,6 +41,8 @@ import org.apache.poi.openxml4j.opc.internal.marshallers.ZipPartMarshaller; 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.openxml4j.util.ZipSecureFile.ThresholdInputStream; import org.apache.poi.util.POILogFactory; import org.apache.poi.util.POILogger; import org.apache.poi.util.TempFile; @@ -85,9 +87,9 @@ public final class ZipPackage extends Package { @SuppressWarnings("deprecation") ZipPackage(InputStream in, PackageAccess access) throws IOException { super(access); - this.zipArchive = new ZipInputStreamZipEntrySource( - new ZipInputStream(in) - ); + InputStream zis = new ZipInputStream(in); + ThresholdInputStream tis = ZipSecureFile.addThreshold(zis); + this.zipArchive = new ZipInputStreamZipEntrySource(tis); } /** diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/ZipHelper.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/ZipHelper.java index 9598b05cbe..408fa416fa 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/ZipHelper.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/ZipHelper.java @@ -29,6 +29,7 @@ import org.apache.poi.openxml4j.exceptions.OpenXML4JException; import org.apache.poi.openxml4j.opc.PackageRelationship; import org.apache.poi.openxml4j.opc.PackageRelationshipTypes; import org.apache.poi.openxml4j.opc.ZipPackage; +import org.apache.poi.openxml4j.util.ZipSecureFile; public final class ZipHelper { @@ -154,7 +155,7 @@ public final class ZipHelper { return null; } - return new ZipFile(file); + return new ZipSecureFile(file); } /** @@ -171,6 +172,6 @@ public final class ZipHelper { return null; } - return new ZipFile(f); + return new ZipSecureFile(f); } } diff --git a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipInputStreamZipEntrySource.java b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipInputStreamZipEntrySource.java index f7f334c75c..7d72212cdd 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipInputStreamZipEntrySource.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipInputStreamZipEntrySource.java @@ -26,6 +26,8 @@ import java.util.Iterator; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; +import org.apache.poi.openxml4j.util.ZipSecureFile.ThresholdInputStream; + /** * Provides a way to get at all the ZipEntries * from a ZipInputStream, as many times as required. @@ -43,7 +45,7 @@ public class ZipInputStreamZipEntrySource implements ZipEntrySource { * We'll then eat lots of memory, but be able to * work with the entries at-will. */ - public ZipInputStreamZipEntrySource(ZipInputStream inp) throws IOException { + public ZipInputStreamZipEntrySource(ThresholdInputStream inp) throws IOException { zipEntries = new ArrayList(); boolean going = true; @@ -105,7 +107,7 @@ public class ZipInputStreamZipEntrySource implements ZipEntrySource { public static class FakeZipEntry extends ZipEntry { private byte[] data; - public FakeZipEntry(ZipEntry entry, ZipInputStream inp) throws IOException { + public FakeZipEntry(ZipEntry entry, InputStream inp) throws IOException { super(entry.getName()); // Grab the de-compressed contents for later diff --git a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java new file mode 100644 index 0000000000..b2f1d82a83 --- /dev/null +++ b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java @@ -0,0 +1,227 @@ +/* ==================================================================== + 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.File; +import java.io.FilterInputStream; +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; +import java.util.zip.ZipFile; +import java.util.zip.ZipInputStream; + +import org.apache.poi.util.POILogFactory; +import org.apache.poi.util.POILogger; + +/** + * This class wraps a {@link ZipFile} in order to check the + * entries for zip bombs + * while reading the archive. + * If a {@link ZipInputStream} is directly used, the wrapper + * can be applied via {@link #addThreshold(InputStream)}. + * The alert limits can be globally defined via {@link #setMaxEntrySize(long)} + * and {@link #setMinInflateRatio(double)}. + */ +public class ZipSecureFile extends ZipFile { + private static POILogger logger = POILogFactory.getLogger(ZipSecureFile.class); + + private static double MIN_INFLATE_RATIO = 0.01d; + private static long MAX_ENTRY_SIZE = 0xFFFFFFFFl; + + /** + * 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 + * + * @param ratio the ratio between de- and inflated bytes to detect zipbomb + */ + public static void setMinInflateRatio(double ratio) { + MIN_INFLATE_RATIO = ratio; + } + + /** + * Sets the maximum file size of a single zip entry. It defaults to 4GB, + * i.e. the 32-bit zip format maximum. + * + * @param maxEntrySize the max. file size of a single zip entry + */ + public static void setMaxEntrySize(long maxEntrySize) { + if (maxEntrySize < 0 || maxEntrySize > 0xFFFFFFFFl) { + throw new IllegalArgumentException("Max entry size is bounded [0-4GB]."); + } + MAX_ENTRY_SIZE = maxEntrySize; + } + + public ZipSecureFile(File file, Charset charset) throws IOException { + super(file, charset); + } + + public ZipSecureFile(File file, int mode, Charset charset) throws IOException { + super(file, mode, charset); + } + + public ZipSecureFile(File file, int mode) throws IOException { + super(file, mode); + } + + public ZipSecureFile(File file) throws ZipException, IOException { + super(file); + } + + public ZipSecureFile(String name, Charset charset) throws IOException { + super(name, charset); + } + + public ZipSecureFile(String name) throws IOException { + super(name); + } + + /** + * Returns an input stream for reading the contents of the specified + * zip file entry. + * + *

Closing this ZIP file will, in turn, close all input + * streams that have been returned by invocations of this method. + * + * @param entry the zip file entry + * @return the input stream for reading the contents of the specified + * zip file entry. + * @throws ZipException if a ZIP format error has occurred + * @throws IOException if an I/O error has occurred + * @throws IllegalStateException if the zip file has been closed + */ + public InputStream getInputStream(ZipEntry entry) throws IOException { + InputStream zipIS = super.getInputStream(entry); + return addThreshold(zipIS); + } + + public static ThresholdInputStream addThreshold(InputStream zipIS) throws IOException { + ThresholdInputStream newInner; + if (zipIS instanceof InflaterInputStream) { + try { + Field f = FilterInputStream.class.getDeclaredField("in"); + f.setAccessible(true); + InputStream oldInner = (InputStream)f.get(zipIS); + newInner = new ThresholdInputStream(oldInner, null); + f.set(zipIS, newInner); + } catch (Exception ex) { + logger.log(POILogger.WARN, "SecurityManager doesn't allow manipulation via reflection for zipbomb detection - continue with original input stream", ex); + newInner = null; + } + } else { + // the inner stream is a ZipFileInputStream, i.e. the data wasn't compressed + newInner = null; + } + + return new ThresholdInputStream(zipIS, newInner); + } + + public static class ThresholdInputStream extends PushbackInputStream { + long counter = 0; + ThresholdInputStream cis; + + public ThresholdInputStream(InputStream is, ThresholdInputStream cis) { + super(is,1); + this.cis = cis; + } + + public int read() throws IOException { + int b = in.read(); + if (b > -1) advance(1); + return b; + } + + public int read(byte b[], int off, int len) throws IOException { + int cnt = in.read(b, off, len); + if (cnt > -1) advance(cnt); + return cnt; + + } + + public long skip(long n) throws IOException { + counter = 0; + return in.skip(n); + } + + public synchronized void reset() throws IOException { + counter = 0; + in.reset(); + } + + public void advance(int advance) throws IOException { + counter += advance; + // check the file size first, in case we are working on uncompressed streams + if (counter < MAX_ENTRY_SIZE) { + if (cis == null) return; + double ratio = (double)cis.counter/(double)counter; + if (ratio > MIN_INFLATE_RATIO) return; + } + throw new IOException("Zip bomb detected! Exiting."); + } + + public ZipEntry getNextEntry() throws IOException { + if (!(in instanceof ZipInputStream)) { + throw new UnsupportedOperationException("underlying stream is not a ZipInputStream"); + } + counter = 0; + return ((ZipInputStream)in).getNextEntry(); + } + + public void closeEntry() throws IOException { + if (!(in instanceof ZipInputStream)) { + throw new UnsupportedOperationException("underlying stream is not a ZipInputStream"); + } + counter = 0; + ((ZipInputStream)in).closeEntry(); + } + + public void unread(int b) throws IOException { + if (!(in instanceof PushbackInputStream)) { + throw new UnsupportedOperationException("underlying stream is not a PushbackInputStream"); + } + if (--counter < 0) counter = 0; + ((PushbackInputStream)in).unread(b); + } + + public void unread(byte[] b, int off, int len) throws IOException { + if (!(in instanceof PushbackInputStream)) { + throw new UnsupportedOperationException("underlying stream is not a PushbackInputStream"); + } + counter -= len; + if (--counter < 0) counter = 0; + ((PushbackInputStream)in).unread(b, off, len); + } + + public int available() throws IOException { + return in.available(); + } + + public boolean markSupported() { + return in.markSupported(); + } + + public synchronized void mark(int readlimit) { + in.mark(readlimit); + } + } +} diff --git a/src/ooxml/java/org/apache/poi/xssf/dev/XSSFDump.java b/src/ooxml/java/org/apache/poi/xssf/dev/XSSFDump.java index 41fc335af0..a3e04eb24c 100644 --- a/src/ooxml/java/org/apache/poi/xssf/dev/XSSFDump.java +++ b/src/ooxml/java/org/apache/poi/xssf/dev/XSSFDump.java @@ -24,6 +24,7 @@ import java.util.Enumeration; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; +import org.apache.poi.openxml4j.opc.internal.ZipHelper; import org.apache.poi.util.IOUtils; import org.apache.xmlbeans.XmlObject; import org.apache.xmlbeans.XmlOptions; @@ -38,7 +39,7 @@ public final class XSSFDump { public static void main(String[] args) throws Exception { for (int i = 0; i < args.length; i++) { System.out.println("Dumping " + args[i]); - ZipFile zip = new ZipFile(args[i]); + ZipFile zip = ZipHelper.openZipFile(args[i]); try { dump(zip); } finally { diff --git a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java index e3a0363ded..bd75179de4 100644 --- a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java +++ b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java @@ -32,6 +32,7 @@ import java.util.zip.ZipFile; import java.util.zip.ZipOutputStream; import org.apache.poi.openxml4j.opc.OPCPackage; +import org.apache.poi.openxml4j.opc.internal.ZipHelper; import org.apache.poi.ss.formula.udf.UDFFinder; import org.apache.poi.ss.usermodel.CellStyle; import org.apache.poi.ss.usermodel.CreationHelper; @@ -334,7 +335,7 @@ public class SXSSFWorkbook implements Workbook } private void injectData(File zipfile, OutputStream out) throws IOException { - ZipFile zip = new ZipFile(zipfile); + ZipFile zip = ZipHelper.openZipFile(zipfile); try { ZipOutputStream zos = new ZipOutputStream(out); diff --git a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/AllOpenXML4JTests.java b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/AllOpenXML4JTests.java index 1dcf93a32d..a1a3ccc887 100644 --- a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/AllOpenXML4JTests.java +++ b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/AllOpenXML4JTests.java @@ -17,28 +17,24 @@ package org.apache.poi.openxml4j.opc; -import junit.framework.Test; -import junit.framework.TestSuite; - import org.apache.poi.openxml4j.opc.compliance.AllOpenXML4JComplianceTests; import org.apache.poi.openxml4j.opc.internal.AllOpenXML4JInternalTests; - +import org.junit.runner.RunWith; +import org.junit.runners.Suite; + +@RunWith(Suite.class) +@Suite.SuiteClasses({ + TestContentType.class + , TestFileHelper.class + , TestListParts.class + , TestPackage.class + , TestPackageCoreProperties.class + , TestPackagePartName.class + , TestPackageThumbnail.class + , TestPackagingURIHelper.class + , TestRelationships.class + , AllOpenXML4JComplianceTests.class + , AllOpenXML4JInternalTests.class +}) public final class AllOpenXML4JTests { - - public static Test suite() { - - TestSuite suite = new TestSuite(AllOpenXML4JTests.class.getName()); - suite.addTestSuite(TestContentType.class); - suite.addTestSuite(TestFileHelper.class); - suite.addTestSuite(TestListParts.class); - suite.addTestSuite(TestPackage.class); - suite.addTestSuite(TestPackageCoreProperties.class); - suite.addTestSuite(TestPackagePartName.class); - suite.addTestSuite(TestPackageThumbnail.class); - suite.addTestSuite(TestPackagingURIHelper.class); - suite.addTestSuite(TestRelationships.class); - suite.addTest(AllOpenXML4JComplianceTests.suite()); - suite.addTest(AllOpenXML4JInternalTests.suite()); - return suite; - } } 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 07a2b333d6..01c1b7707d 100644 --- a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java +++ b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java @@ -17,6 +17,14 @@ package org.apache.poi.openxml4j.opc; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileInputStream; @@ -25,35 +33,47 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.lang.reflect.Field; +import java.lang.reflect.InvocationTargetException; import java.net.URI; +import java.util.Enumeration; import java.util.HashMap; import java.util.List; import java.util.TreeMap; import java.util.regex.Pattern; +import java.util.zip.ZipEntry; +import java.util.zip.ZipFile; +import java.util.zip.ZipOutputStream; -import junit.framework.TestCase; - +import org.apache.poi.POIXMLException; import org.apache.poi.openxml4j.OpenXML4JTestDataSamples; import org.apache.poi.openxml4j.exceptions.InvalidFormatException; import org.apache.poi.openxml4j.exceptions.InvalidOperationException; import org.apache.poi.openxml4j.opc.internal.ContentTypeManager; import org.apache.poi.openxml4j.opc.internal.FileHelper; import org.apache.poi.openxml4j.opc.internal.PackagePropertiesPart; +import org.apache.poi.openxml4j.opc.internal.ZipHelper; +import org.apache.poi.openxml4j.util.ZipSecureFile; +import org.apache.poi.ss.usermodel.Workbook; +import org.apache.poi.ss.usermodel.WorkbookFactory; import org.apache.poi.util.DocumentHelper; +import org.apache.poi.util.IOUtils; import org.apache.poi.util.POILogFactory; import org.apache.poi.util.POILogger; import org.apache.poi.util.TempFile; +import org.junit.Ignore; +import org.junit.Test; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.NodeList; -public final class TestPackage extends TestCase { +public final class TestPackage { private static final POILogger logger = POILogFactory.getLogger(TestPackage.class); /** * Test that just opening and closing the file doesn't alter the document. */ - public void testOpenSave() throws Exception { + @Test + public void openSave() throws Exception { String originalFile = OpenXML4JTestDataSamples.getSampleFileName("TestPackageCommon.docx"); File targetFile = OpenXML4JTestDataSamples.getOutputFile("TestPackageOpenSaveTMP.docx"); @@ -75,7 +95,8 @@ public final class TestPackage extends TestCase { * Test that when we create a new Package, we give it * the correct default content types */ - public void testCreateGetsContentTypes() throws Exception { + @Test + public void createGetsContentTypes() throws Exception { File targetFile = OpenXML4JTestDataSamples.getOutputFile("TestCreatePackageTMP.docx"); // Zap the target file, in case of an earlier run @@ -107,7 +128,8 @@ public final class TestPackage extends TestCase { /** * Test package creation. */ - public void testCreatePackageAddPart() throws Exception { + @Test + public void createPackageAddPart() throws Exception { File targetFile = OpenXML4JTestDataSamples.getOutputFile("TestCreatePackageTMP.docx"); File expectedFile = OpenXML4JTestDataSamples.getSampleFile("TestCreatePackageOUTPUT.docx"); @@ -153,7 +175,8 @@ public final class TestPackage extends TestCase { * document and another part, save and re-load and * have everything setup as expected */ - public void testCreatePackageWithCoreDocument() throws Exception { + @Test + public void createPackageWithCoreDocument() throws Exception { ByteArrayOutputStream baos = new ByteArrayOutputStream(); OPCPackage pkg = OPCPackage.create(baos); @@ -247,7 +270,8 @@ public final class TestPackage extends TestCase { /** * Test package opening. */ - public void testOpenPackage() throws Exception { + @Test + public void openPackage() throws Exception { File targetFile = OpenXML4JTestDataSamples.getOutputFile("TestOpenPackageTMP.docx"); File inputFile = OpenXML4JTestDataSamples.getSampleFile("TestOpenPackageINPUT.docx"); @@ -306,7 +330,8 @@ public final class TestPackage extends TestCase { * OutputStream, in addition to the normal writing * to a file */ - public void testSaveToOutputStream() throws Exception { + @Test + public void saveToOutputStream() throws Exception { String originalFile = OpenXML4JTestDataSamples.getSampleFileName("TestPackageCommon.docx"); File targetFile = OpenXML4JTestDataSamples.getOutputFile("TestPackageOpenSaveTMP.docx"); @@ -334,7 +359,8 @@ public final class TestPackage extends TestCase { * simple InputStream, in addition to the normal * reading from a file */ - public void testOpenFromInputStream() throws Exception { + @Test + public void openFromInputStream() throws Exception { String originalFile = OpenXML4JTestDataSamples.getSampleFileName("TestPackageCommon.docx"); FileInputStream finp = new FileInputStream(originalFile); @@ -353,7 +379,9 @@ public final class TestPackage extends TestCase { /** * TODO: fix and enable */ - public void disabled_testRemovePartRecursive() throws Exception { + @Test + @Ignore + public void removePartRecursive() throws Exception { String originalFile = OpenXML4JTestDataSamples.getSampleFileName("TestPackageCommon.docx"); File targetFile = OpenXML4JTestDataSamples.getOutputFile("TestPackageRemovePartRecursiveOUTPUT.docx"); File tempFile = OpenXML4JTestDataSamples.getOutputFile("TestPackageRemovePartRecursiveTMP.docx"); @@ -369,7 +397,8 @@ public final class TestPackage extends TestCase { assertTrue(targetFile.delete()); } - public void testDeletePart() throws InvalidFormatException { + @Test + public void deletePart() throws InvalidFormatException { TreeMap expectedValues; TreeMap values; @@ -426,7 +455,8 @@ public final class TestPackage extends TestCase { p.revert(); } - public void testDeletePartRecursive() throws InvalidFormatException { + @Test + public void deletePartRecursive() throws InvalidFormatException { TreeMap expectedValues; TreeMap values; @@ -468,7 +498,8 @@ public final class TestPackage extends TestCase { * Test that we can open a file by path, and then * write changes to it. */ - public void testOpenFileThenOverwrite() throws Exception { + @Test + public void openFileThenOverwrite() throws Exception { File tempFile = TempFile.createTempFile("poiTesting","tmp"); File origFile = OpenXML4JTestDataSamples.getSampleFile("TestPackageCommon.docx"); FileHelper.copyFile(origFile, tempFile); @@ -505,7 +536,8 @@ public final class TestPackage extends TestCase { * Test that we can open a file by path, save it * to another file, then delete both */ - public void testOpenFileThenSaveDelete() throws Exception { + @Test + public void openFileThenSaveDelete() throws Exception { File tempFile = TempFile.createTempFile("poiTesting","tmp"); File tempFile2 = TempFile.createTempFile("poiTesting","tmp"); File origFile = OpenXML4JTestDataSamples.getSampleFile("TestPackageCommon.docx"); @@ -529,7 +561,8 @@ public final class TestPackage extends TestCase { return (ContentTypeManager)f.get(pkg); } - public void testGetPartsByName() throws Exception { + @Test + public void getPartsByName() throws Exception { String filepath = OpenXML4JTestDataSamples.getSampleFileName("sample.docx"); OPCPackage pkg = OPCPackage.open(filepath, PackageAccess.READ_WRITE); @@ -553,7 +586,8 @@ public final class TestPackage extends TestCase { } } - public void testGetPartSize() throws Exception { + @Test + public void getPartSize() throws Exception { String filepath = OpenXML4JTestDataSamples.getSampleFileName("sample.docx"); OPCPackage pkg = OPCPackage.open(filepath, PackageAccess.READ); try { @@ -585,7 +619,8 @@ public final class TestPackage extends TestCase { } } - public void testReplaceContentType() throws Exception { + @Test + public void replaceContentType() throws Exception { InputStream is = OpenXML4JTestDataSamples.openSampleStream("sample.xlsx"); OPCPackage p = OPCPackage.open(is); @@ -603,4 +638,113 @@ public final class TestPackage extends TestCase { assertFalse(mgr.isContentTypeRegister("application/vnd.openxmlformats-officedocument.spreadsheetml.sheet.main+xml")); assertTrue(mgr.isContentTypeRegister("application/vnd.ms-excel.sheet.macroEnabled.main+xml")); } + + @Test(expected=IOException.class) + public void zipBombCreateAndHandle() throws Exception { + // #50090 / #56865 + ZipFile zipFile = ZipHelper.openZipFile(OpenXML4JTestDataSamples.getSampleFile("sample.xlsx")); + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + ZipOutputStream append = new ZipOutputStream(bos); + // first, copy contents from existing war + Enumeration entries = zipFile.entries(); + while (entries.hasMoreElements()) { + ZipEntry e2 = entries.nextElement(); + ZipEntry e = new ZipEntry(e2.getName()); + e.setTime(e2.getTime()); + e.setComment(e2.getComment()); + e.setSize(e2.getSize()); + + append.putNextEntry(e); + if (!e.isDirectory()) { + InputStream is = zipFile.getInputStream(e); + if (e.getName().equals("[Content_Types].xml")) { + ByteArrayOutputStream bos2 = new ByteArrayOutputStream(); + IOUtils.copy(is, bos2); + long size = bos2.size()-"".length(); + append.write(bos2.toByteArray(), 0, (int)size); + byte spam[] = new byte[0x7FFF]; + for (int i=0; i".getBytes()); + size += 8; + e.setSize(size); + } else { + IOUtils.copy(is, append); + } + } + append.closeEntry(); + } + + append.close(); + zipFile.close(); + + byte buf[] = bos.toByteArray(); + bos = null; + + Workbook wb = WorkbookFactory.create(new ByteArrayInputStream(buf)); + wb.getSheetAt(0); + wb.close(); + } + + @Test + public void zipBombCheckSizes() throws Exception { + File file = OpenXML4JTestDataSamples.getSampleFile("sample.xlsx"); + + try { + double min_ratio = Double.MAX_VALUE; + long max_size = 0; + ZipFile zf = ZipHelper.openZipFile(file); + Enumeration entries = zf.entries(); + while (entries.hasMoreElements()) { + ZipEntry ze = entries.nextElement(); + double ratio = (double)ze.getCompressedSize() / (double)ze.getSize(); + min_ratio = Math.min(min_ratio, ratio); + max_size = Math.max(max_size, ze.getSize()); + } + zf.close(); + + // use values close to, but within the limits + ZipSecureFile.setMinInflateRatio(min_ratio-0.002); + ZipSecureFile.setMaxEntrySize(max_size+1); + Workbook wb = WorkbookFactory.create(file); + wb.close(); + + // check ratio ouf of bounds + ZipSecureFile.setMinInflateRatio(min_ratio+0.002); + try { + wb = WorkbookFactory.create(file); + wb.close(); + // this is a bit strange, as there will be different exceptions thrown + // depending if this executed via "ant test" or within eclipse + // maybe a difference in JDK ... + } catch (InvalidFormatException e) { + assertEquals("Zip bomb detected! Exiting.", e.getMessage()); + } catch (POIXMLException e) { + InvocationTargetException t = (InvocationTargetException)e.getCause(); + IOException t2 = (IOException)t.getTargetException(); + assertEquals("Zip bomb detected! Exiting.", t2.getMessage()); + } + + // check max entry size ouf of bounds + ZipSecureFile.setMinInflateRatio(min_ratio-0.002); + ZipSecureFile.setMaxEntrySize(max_size-1); + try { + wb = WorkbookFactory.create(file, null, true); + wb.close(); + } catch (InvalidFormatException e) { + assertEquals("Zip bomb detected! Exiting.", e.getMessage()); + } catch (POIXMLException e) { + InvocationTargetException t = (InvocationTargetException)e.getCause(); + IOException t2 = (IOException)t.getTargetException(); + assertEquals("Zip bomb detected! Exiting.", t2.getMessage()); + } + } finally { + // reset otherwise a lot of ooxml tests will fail + ZipSecureFile.setMinInflateRatio(0.01d); + ZipSecureFile.setMaxEntrySize(0xFFFFFFFFl); + } + } } diff --git a/src/testcases/org/apache/poi/ss/formula/function/ExcelFileFormatDocFunctionExtractor.java b/src/testcases/org/apache/poi/ss/formula/function/ExcelFileFormatDocFunctionExtractor.java index 13581c8a4f..617f7b052d 100644 --- a/src/testcases/org/apache/poi/ss/formula/function/ExcelFileFormatDocFunctionExtractor.java +++ b/src/testcases/org/apache/poi/ss/formula/function/ExcelFileFormatDocFunctionExtractor.java @@ -516,6 +516,7 @@ public final class ExcelFileFormatDocFunctionExtractor { ps.println("#Columns: (index, name, minParams, maxParams, returnClass, paramClasses, isVolatile, hasFootnote )"); ps.println(""); try { + // can't use ZipHelper here, because its in a different module ZipFile zf = new ZipFile(effDocFile); InputStream is = zf.getInputStream(zf.getEntry("content.xml")); extractFunctionData(new FunctionDataCollector(ps), is); -- 2.39.5