]> source.dussan.org Git - poi.git/commitdiff
Bug 58499: Don't report Zip-Bomb for small files which should not cause memory issues...
authorDominik Stadler <centic@apache.org>
Sat, 17 Oct 2015 14:40:36 +0000 (14:40 +0000)
committerDominik Stadler <centic@apache.org>
Sat, 17 Oct 2015 14:40:36 +0000 (14:40 +0000)
Also make error message a bit more specific and list classname in Zip-Bomb-Error to make it easier for users what the
problem is and how to find out where the static methods are

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

src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java
src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java
src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java
src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java

index 5423ea4d475603450c80794aba8cb8d69ce58cb7..b06ddcb25a72a594ade17eacce515683202af79a 100644 (file)
@@ -46,6 +46,9 @@ public class ZipSecureFile extends ZipFile {
     \r
     private static double MIN_INFLATE_RATIO = 0.01d;\r
     private static long MAX_ENTRY_SIZE = 0xFFFFFFFFl;\r
+    \r
+    // don't alert for expanded sizes smaller than 100k\r
+    private static long GRACE_ENTRY_SIZE = 100*1024;\r
 \r
     /**\r
      * Sets the ratio between de- and inflated bytes to detect zipbomb.\r
@@ -183,16 +186,37 @@ public class ZipSecureFile extends ZipFile {
 \r
         public void advance(int advance) throws IOException {\r
             counter += advance;\r
+            \r
             // check the file size first, in case we are working on uncompressed streams\r
-            if (counter < MAX_ENTRY_SIZE) {\r
-                if (cis == null) return;\r
-                double ratio = (double)cis.counter/(double)counter;\r
-                if (ratio >= MIN_INFLATE_RATIO) return;\r
+            if(counter > MAX_ENTRY_SIZE) {\r
+                throw new IOException("Zip bomb detected! The file would exceed the max size of the expanded data in the zip-file. "\r
+                        + "This may indicates that the file is used to inflate memory usage and thus could pose a security risk. "\r
+                        + "You can adjust this limit via ZipSecureFile.setMaxEntrySize() if you need to work with files which are very large. "\r
+                        + "Counter: " + counter + ", cis.counter: " + (cis == null ? 0 : cis.counter)\r
+                        + "Limits: MAX_ENTRY_SIZE: " + MAX_ENTRY_SIZE);\r
+            }\r
+\r
+            // no expanded size?\r
+            if (cis == null) {\r
+                return;\r
+            }\r
+            \r
+            // don't alert for small expanded size\r
+            if (counter <= GRACE_ENTRY_SIZE) {\r
+                return;\r
             }\r
-            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. "\r
-                    + "You can adjust these limits via setMinInflateRatio() and setMaxEntrySize() if you need to work with files which exceed these limits. "\r
+\r
+            double ratio = (double)cis.counter/(double)counter;\r
+            if (ratio >= MIN_INFLATE_RATIO) {\r
+                return;\r
+            }\r
+\r
+            // one of the limits was reached, report it\r
+            throw new IOException("Zip bomb detected! The file would exceed the max. ratio of compressed file size to the size of the expanded data. "\r
+                    + "This may indicate that the file is used to inflate memory usage and thus could pose a security risk. "\r
+                    + "You can adjust this limit via ZipSecureFile.setMinInflateRatio() if you need to work with files which exceed this limit. "\r
                     + "Counter: " + counter + ", cis.counter: " + (cis == null ? 0 : cis.counter) + ", ratio: " + (cis == null ? 0 : ((double)cis.counter)/counter)\r
-                    + "Limits: MIN_INFLATE_RATIO: " + MIN_INFLATE_RATIO + ", MAX_ENTRY_SIZE: " + MAX_ENTRY_SIZE);\r
+                    + "Limits: MIN_INFLATE_RATIO: " + MIN_INFLATE_RATIO);\r
         }\r
 \r
         public ZipEntry getNextEntry() throws IOException {\r
index 2cbca15520f61b981756547578646e4748c4eecc..6540e049d571cdb3208923184aaa75bcc59e0176 100644 (file)
@@ -28,7 +28,6 @@ import static org.junit.Assert.fail;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
-import java.io.OutputStream;
 import java.lang.reflect.Field;
 
 import org.apache.poi.ss.usermodel.BaseTestWorkbook;
@@ -209,7 +208,7 @@ public final class TestSXSSFWorkbook extends BaseTestWorkbook {
     }
 
     @Test
-    public void sheetdataWriter(){
+    public void sheetdataWriter() throws IOException{
         SXSSFWorkbook wb = new SXSSFWorkbook();
         SXSSFSheet sh = wb.createSheet();
         SheetDataWriter wr = sh.getSheetDataWriter();
@@ -218,6 +217,7 @@ public final class TestSXSSFWorkbook extends BaseTestWorkbook {
         assertTrue(tmp.getName().startsWith("poi-sxssf-sheet"));
         assertTrue(tmp.getName().endsWith(".xml"));
         assertTrue(wb.dispose());
+        wb.close();
 
         wb = new SXSSFWorkbook();
         wb.setCompressTempFiles(true);
@@ -228,6 +228,7 @@ public final class TestSXSSFWorkbook extends BaseTestWorkbook {
         assertTrue(tmp.getName().startsWith("poi-sxssf-sheet-xml"));
         assertTrue(tmp.getName().endsWith(".gz"));
         assertTrue(wb.dispose());
+        wb.close();
 
         //Test escaping of Unicode control characters
         wb = new SXSSFWorkbook();
@@ -237,6 +238,7 @@ public final class TestSXSSFWorkbook extends BaseTestWorkbook {
         assertEquals("value?", cell.getStringCellValue());
 
         assertTrue(wb.dispose());
+        wb.close();
 
     }
 
@@ -329,80 +331,74 @@ public final class TestSXSSFWorkbook extends BaseTestWorkbook {
 
     // currently writing the same sheet multiple times is not supported...
     @Ignore
-       public void bug53515() throws Exception {
-               Workbook wb = new SXSSFWorkbook(10);
-               populateWorkbook(wb);
-               saveTwice(wb);
-               wb = new XSSFWorkbook();
-               populateWorkbook(wb);
-               saveTwice(wb);
-       }
+    public void bug53515() throws Exception {
+        Workbook wb = new SXSSFWorkbook(10);
+        populateWorkbook(wb);
+        saveTwice(wb);
+        wb = new XSSFWorkbook();
+        populateWorkbook(wb);
+        saveTwice(wb);
+    }
 
-       // Crashes the JVM because of documented JVM behavior with concurrent writing/reading of zip-files
-       // See http://www.oracle.com/technetwork/java/javase/documentation/overview-156328.html
+    // Crashes the JVM because of documented JVM behavior with concurrent writing/reading of zip-files
+    // See http://www.oracle.com/technetwork/java/javase/documentation/overview-156328.html
     @Ignore
-       public void bug53515a() throws Exception {
-               File out = new File("Test.xlsx");
-               out.delete();
-               for (int i = 0; i < 2; i++) {
-                       System.out.println("Iteration " + i);
-                       final SXSSFWorkbook wb;
-                       if (out.exists()) {
-                               wb = new SXSSFWorkbook(
-                                               (XSSFWorkbook) WorkbookFactory.create(out));
-                       } else {
-                               wb = new SXSSFWorkbook(10);
-                       }
-
-                       try {
-                               FileOutputStream outSteam = new FileOutputStream(out);
-                               if (i == 0) {
-                                       populateWorkbook(wb);
-                               } else {
-                                       System.gc();
-                                       System.gc();
-                                       System.gc();
-                               }
-
-                               wb.write(outSteam);
-                               // assertTrue(wb.dispose());
-                               outSteam.close();
-                       } finally {
-                               assertTrue(wb.dispose());
-                       }
-               }
-               out.delete();
-       }
+    public void bug53515a() throws Exception {
+        File out = new File("Test.xlsx");
+        out.delete();
+        for (int i = 0; i < 2; i++) {
+            System.out.println("Iteration " + i);
+            final SXSSFWorkbook wb;
+            if (out.exists()) {
+                wb = new SXSSFWorkbook(
+                        (XSSFWorkbook) WorkbookFactory.create(out));
+            } else {
+                wb = new SXSSFWorkbook(10);
+            }
 
-       private static void populateWorkbook(Workbook wb) {
-               Sheet sh = wb.createSheet();
-               for (int rownum = 0; rownum < 100; rownum++) {
-                       Row row = sh.createRow(rownum);
-                       for (int cellnum = 0; cellnum < 10; cellnum++) {
-                               Cell cell = row.createCell(cellnum);
-                               String address = new CellReference(cell).formatAsString();
-                               cell.setCellValue(address);
-                       }
-               }
-       }
+            try {
+                FileOutputStream outSteam = new FileOutputStream(out);
+                if (i == 0) {
+                    populateWorkbook(wb);
+                } else {
+                    System.gc();
+                    System.gc();
+                    System.gc();
+                }
+
+                wb.write(outSteam);
+                // assertTrue(wb.dispose());
+                outSteam.close();
+            } finally {
+                assertTrue(wb.dispose());
+            }
+        }
+        out.delete();
+    }
 
-       private static void saveTwice(Workbook wb) throws Exception {
-               for (int i = 0; i < 2; i++) {
-                       try {
-                               NullOutputStream out = new NullOutputStream();
-                               wb.write(out);
-                               out.close();
-                       } catch (Exception e) {
-                               throw new Exception("ERROR: failed on " + (i + 1)
-                                               + "th time calling " + wb.getClass().getName()
-                                               + ".write() with exception " + e.getMessage(), e);
-                       }
-               }
-       }
+    private static void populateWorkbook(Workbook wb) {
+        Sheet sh = wb.createSheet();
+        for (int rownum = 0; rownum < 100; rownum++) {
+            Row row = sh.createRow(rownum);
+            for (int cellnum = 0; cellnum < 10; cellnum++) {
+                Cell cell = row.createCell(cellnum);
+                String address = new CellReference(cell).formatAsString();
+                cell.setCellValue(address);
+            }
+        }
+    }
 
-       private static class NullOutputStream extends OutputStream {
-               @Override
-               public void write(int b) throws IOException {
-               }
-       }
+    private static void saveTwice(Workbook wb) throws Exception {
+        for (int i = 0; i < 2; i++) {
+            try {
+                NullOutputStream out = new NullOutputStream();
+                wb.write(out);
+                out.close();
+            } catch (Exception e) {
+                throw new Exception("ERROR: failed on " + (i + 1)
+                        + "th time calling " + wb.getClass().getName()
+                        + ".write() with exception " + e.getMessage(), e);
+            }
+        }
+    }
 }
index f40ce344f200fba239b480caa46971392d39ac32..4b92b30bc2c74f3ea5ccb18c5cc6ffb1ffd65d62 100644 (file)
@@ -81,19 +81,6 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
         return wb.getWorkbook();
     }
 
-    @Test
-    public void windowOneDefaults() throws IOException {
-        HSSFWorkbook b = new HSSFWorkbook( );
-        try {
-            assertEquals(b.getActiveSheetIndex(), 0);
-            assertEquals(b.getFirstVisibleTab(), 0);
-        } catch (NullPointerException npe) {
-            fail("WindowOneRecord in Workbook is probably not initialized");
-        }
-        
-        b.close();
-    }
-
     /**
      * Tests for {@link HSSFWorkbook#isHidden()} etc
      * @throws IOException 
index 4a9dd65c09596af0741064f67bf5e1837632a40f..ca69fa626670e40f1dd52f156346be7c25163291 100644 (file)
@@ -26,15 +26,16 @@ import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import java.io.IOException;
+import java.io.OutputStream;
 import java.util.ConcurrentModificationException;
 import java.util.Iterator;
 
-import junit.framework.AssertionFailedError;
-
 import org.apache.poi.ss.ITestDataProvider;
 import org.apache.poi.ss.util.CellRangeAddress;
 import org.junit.Test;
 
+import junit.framework.AssertionFailedError;
+
 /**
  * @author Yegor Kozlov
  */
@@ -753,4 +754,48 @@ public abstract class BaseTestWorkbook {
                                        sheets[i], wb.getSheetAt(i).getSheetName());
                }
        }
+
+    protected static class NullOutputStream extends OutputStream {
+        public NullOutputStream() {
+        }
+
+        @Override
+        public void write(int b) throws IOException {
+        }
+    }
+
+    @Test
+    public void test58499() throws IOException {
+        Workbook workbook = _testDataProvider.createWorkbook();
+        Sheet sheet = workbook.createSheet();
+        for (int i = 0; i < 900; i++) {
+            Row r = sheet.createRow(i);
+            Cell c = r.createCell(0);
+            CellStyle cs = workbook.createCellStyle();
+            c.setCellStyle(cs);
+            c.setCellValue("AAA");                
+        }
+        OutputStream os = new NullOutputStream();
+        try {
+            workbook.write(os);
+        } finally {
+            os.close();
+        }
+        //workbook.dispose();
+        workbook.close();
+    }
+
+
+    @Test
+    public void windowOneDefaults() throws IOException {
+        Workbook b = _testDataProvider.createWorkbook();
+        try {
+            assertEquals(b.getActiveSheetIndex(), 0);
+            assertEquals(b.getFirstVisibleTab(), 0);
+        } catch (NullPointerException npe) {
+            fail("WindowOneRecord in Workbook is probably not initialized");
+        }
+        
+        b.close();
+    }
 }