You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by ce...@apache.org on 2015/10/17 16:40:36 UTC
svn commit: r1709180 - in /poi/trunk/src:
ooxml/java/org/apache/poi/openxml4j/util/
ooxml/testcases/org/apache/poi/xssf/streaming/
testcases/org/apache/poi/hssf/usermodel/
testcases/org/apache/poi/ss/usermodel/
Author: centic
Date: Sat Oct 17 14:40:36 2015
New Revision: 1709180
URL: http://svn.apache.org/viewvc?rev=1709180&view=rev
Log:
Bug 58499: Don't report Zip-Bomb for small files which should not cause memory issues anyway
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
Modified:
poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java
poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java
poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java
poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java
Modified: poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java?rev=1709180&r1=1709179&r2=1709180&view=diff
==============================================================================
--- poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java (original)
+++ poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java Sat Oct 17 14:40:36 2015
@@ -46,6 +46,9 @@ public class ZipSecureFile extends ZipFi
private static double MIN_INFLATE_RATIO = 0.01d;
private static long MAX_ENTRY_SIZE = 0xFFFFFFFFl;
+
+ // don't alert for expanded sizes smaller than 100k
+ private static long GRACE_ENTRY_SIZE = 100*1024;
/**
* Sets the ratio between de- and inflated bytes to detect zipbomb.
@@ -183,16 +186,37 @@ public class ZipSecureFile extends ZipFi
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;
+ if(counter > MAX_ENTRY_SIZE) {
+ throw new IOException("Zip bomb detected! The file would exceed the max size of the expanded data in the zip-file. "
+ + "This may indicates that the file is used to inflate memory usage and thus could pose a security risk. "
+ + "You can adjust this limit via ZipSecureFile.setMaxEntrySize() if you need to work with files which are very large. "
+ + "Counter: " + counter + ", cis.counter: " + (cis == null ? 0 : cis.counter)
+ + "Limits: MAX_ENTRY_SIZE: " + MAX_ENTRY_SIZE);
+ }
+
+ // no expanded size?
+ if (cis == null) {
+ return;
+ }
+
+ // don't alert for small expanded size
+ if (counter <= GRACE_ENTRY_SIZE) {
+ return;
}
- 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. "
+
+ double ratio = (double)cis.counter/(double)counter;
+ if (ratio >= MIN_INFLATE_RATIO) {
+ return;
+ }
+
+ // one of the limits was reached, report it
+ throw new IOException("Zip bomb detected! The file would exceed the max. ratio of compressed file size to the size of the expanded data. "
+ + "This may indicate that the file is used to inflate memory usage and thus could pose a security risk. "
+ + "You can adjust this limit via ZipSecureFile.setMinInflateRatio() if you need to work with files which exceed this limit. "
+ "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);
+ + "Limits: MIN_INFLATE_RATIO: " + MIN_INFLATE_RATIO);
}
public ZipEntry getNextEntry() throws IOException {
Modified: poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java?rev=1709180&r1=1709179&r2=1709180&view=diff
==============================================================================
--- poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java (original)
+++ poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java Sat Oct 17 14:40:36 2015
@@ -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 ext
}
@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 ext
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 ext
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 ext
assertEquals("value?", cell.getStringCellValue());
assertTrue(wb.dispose());
+ wb.close();
}
@@ -329,80 +331,74 @@ public final class TestSXSSFWorkbook ext
// 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);
+ }
+ }
+ }
}
Modified: poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java?rev=1709180&r1=1709179&r2=1709180&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java Sat Oct 17 14:40:36 2015
@@ -81,19 +81,6 @@ public final class TestHSSFWorkbook exte
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
Modified: poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java?rev=1709180&r1=1709179&r2=1709180&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java Sat Oct 17 14:40:36 2015
@@ -26,15 +26,16 @@ import static org.junit.Assert.assertTru
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();
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org