diff options
author | Dominik Stadler <centic@apache.org> | 2023-08-07 20:01:19 +0000 |
---|---|---|
committer | Dominik Stadler <centic@apache.org> | 2023-08-07 20:01:19 +0000 |
commit | 163ff25594bb2751ea2ea5e3df4f82fdf9219304 (patch) | |
tree | e060a2367bc285df2d7138ed1db10ed8e942b558 | |
parent | f3997b49efaef3987f87e212ec41635b4339bbf9 (diff) | |
download | poi-163ff25594bb2751ea2ea5e3df4f82fdf9219304.tar.gz poi-163ff25594bb2751ea2ea5e3df4f82fdf9219304.zip |
Bug 66425: Avoid a ClassCastException found via oss-fuzz
We try to avoid throwing NullPointerException, but it was possible
to trigger one here with a specially crafted input-file
Should fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=61266
git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1911523 13f79535-47bb-0310-9956-ffa450edef68
-rw-r--r-- | poi-examples/src/test/java/org/apache/poi/integration/TestXLSX2CSV.java | 21 | ||||
-rw-r--r-- | poi-integration/src/test/java/org/apache/poi/stress/TestAllFiles.java | 2 | ||||
-rw-r--r-- | poi-ooxml/src/main/java/org/apache/poi/xssf/eventusermodel/ReadOnlySharedStringsTable.java | 12 | ||||
-rw-r--r-- | poi-ooxml/src/test/java/org/apache/poi/xssf/eventusermodel/TestReadOnlySharedStringsTable.java | 85 | ||||
-rw-r--r-- | test-data/spreadsheet/clusterfuzz-testcase-minimized-XLSX2CSVFuzzer-5025401116950528.xlsx | bin | 0 -> 5615 bytes | |||
-rw-r--r-- | test-data/spreadsheet/stress.xls | bin | 60928 -> 61440 bytes |
6 files changed, 83 insertions, 37 deletions
diff --git a/poi-examples/src/test/java/org/apache/poi/integration/TestXLSX2CSV.java b/poi-examples/src/test/java/org/apache/poi/integration/TestXLSX2CSV.java index b172b38295..0867f12b81 100644 --- a/poi-examples/src/test/java/org/apache/poi/integration/TestXLSX2CSV.java +++ b/poi-examples/src/test/java/org/apache/poi/integration/TestXLSX2CSV.java @@ -18,6 +18,7 @@ package org.apache.poi.integration; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.PrintStream; @@ -26,6 +27,7 @@ import java.nio.charset.StandardCharsets; import org.apache.commons.io.output.UnsynchronizedByteArrayOutputStream; import org.apache.poi.examples.xssf.eventusermodel.XLSX2CSV; +import org.apache.poi.ooxml.POIXMLException; import org.apache.poi.openxml4j.opc.OPCPackage; import org.apache.poi.openxml4j.opc.PackageAccess; import org.apache.poi.xssf.XSSFTestDataSamples; @@ -95,6 +97,25 @@ public class TestXLSX2CSV { } @Test + public void testInvalidSampleFile() throws Exception { + final UnsynchronizedByteArrayOutputStream outputBytes = UnsynchronizedByteArrayOutputStream.builder().get(); + PrintStream out = new PrintStream(outputBytes, true, StandardCharsets.UTF_8.name()); + + // The package open is instantaneous, as it should be. + try (OPCPackage p = OPCPackage.open(XSSFTestDataSamples.getSampleFile("clusterfuzz-testcase-minimized-XLSX2CSVFuzzer-5025401116950528.xlsx").getAbsolutePath(), PackageAccess.READ)) { + XLSX2CSV xlsx2csv = new XLSX2CSV(p, out, -1); + assertThrows(POIXMLException.class, + xlsx2csv::process); + } + + String errorOutput = errorBytes.toString(StandardCharsets.UTF_8); + assertEquals("", errorOutput); + + String output = outputBytes.toString(StandardCharsets.UTF_8); + assertEquals("", output, "Had: " + output); + } + + @Test public void testMinColumns() throws Exception { final UnsynchronizedByteArrayOutputStream outputBytes = UnsynchronizedByteArrayOutputStream.builder().get(); PrintStream out = new PrintStream(outputBytes, true, StandardCharsets.UTF_8.name()); diff --git a/poi-integration/src/test/java/org/apache/poi/stress/TestAllFiles.java b/poi-integration/src/test/java/org/apache/poi/stress/TestAllFiles.java index 446375affd..a9f3b37aee 100644 --- a/poi-integration/src/test/java/org/apache/poi/stress/TestAllFiles.java +++ b/poi-integration/src/test/java/org/apache/poi/stress/TestAllFiles.java @@ -235,7 +235,7 @@ public class TestAllFiles { @SuppressWarnings("unchecked") private static void verify(String file, Executable exec, Class<? extends Throwable> exClass, String exMessage, String password, FileHandler fileHandler) { - final String errPrefix = file + " - failed for handler " + fileHandler.getClass().getSimpleName() + ": "; + final String errPrefix = file.replace("\\", "/") + " - failed for handler " + fileHandler.getClass().getSimpleName() + ": "; // this also removes the password for non encrypted files Biff8EncryptionKey.setCurrentUserPassword(password); if (exClass != null && AssertionFailedError.class.isAssignableFrom(exClass)) { diff --git a/poi-ooxml/src/main/java/org/apache/poi/xssf/eventusermodel/ReadOnlySharedStringsTable.java b/poi-ooxml/src/main/java/org/apache/poi/xssf/eventusermodel/ReadOnlySharedStringsTable.java index 6336836821..07d00bd99a 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/xssf/eventusermodel/ReadOnlySharedStringsTable.java +++ b/poi-ooxml/src/main/java/org/apache/poi/xssf/eventusermodel/ReadOnlySharedStringsTable.java @@ -251,7 +251,9 @@ public class ReadOnlySharedStringsTable extends DefaultHandler implements Shared this.strings = new ArrayList<>(this.uniqueCount); characters = new StringBuilder(64); } else if ("si".equals(localName)) { - characters.setLength(0); + if (characters != null) { + characters.setLength(0); + } } else if ("t".equals(localName)) { tIsOpen = true; } else if ("rPh".equals(localName)) { @@ -269,7 +271,9 @@ public class ReadOnlySharedStringsTable extends DefaultHandler implements Shared } if ("si".equals(localName)) { - strings.add(characters.toString()); + if (strings != null && characters != null) { + strings.add(characters.toString()); + } } else if ("t".equals(localName)) { tIsOpen = false; } else if ("rPh".equals(localName)) { @@ -285,7 +289,9 @@ public class ReadOnlySharedStringsTable extends DefaultHandler implements Shared if (inRPh && includePhoneticRuns) { characters.append(ch, start, length); } else if (! inRPh){ - characters.append(ch, start, length); + if (characters != null) { + characters.append(ch, start, length); + } } } } diff --git a/poi-ooxml/src/test/java/org/apache/poi/xssf/eventusermodel/TestReadOnlySharedStringsTable.java b/poi-ooxml/src/test/java/org/apache/poi/xssf/eventusermodel/TestReadOnlySharedStringsTable.java index 7df35d713c..be486270ed 100644 --- a/poi-ooxml/src/test/java/org/apache/poi/xssf/eventusermodel/TestReadOnlySharedStringsTable.java +++ b/poi-ooxml/src/test/java/org/apache/poi/xssf/eventusermodel/TestReadOnlySharedStringsTable.java @@ -20,6 +20,7 @@ package org.apache.poi.xssf.eventusermodel; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import java.io.IOException; import java.io.InputStream; @@ -39,7 +40,7 @@ import org.xml.sax.SAXException; * Tests for {@link org.apache.poi.xssf.eventusermodel.XSSFReader} */ public final class TestReadOnlySharedStringsTable { - private static POIDataSamples _ssTests = POIDataSamples.getSpreadSheetInstance(); + private static final POIDataSamples _ssTests = POIDataSamples.getSpreadSheetInstance(); @Test void testParse() throws Exception { @@ -47,24 +48,25 @@ public final class TestReadOnlySharedStringsTable { List<PackagePart> parts = pkg.getPartsByName(Pattern.compile("/xl/sharedStrings.xml")); assertEquals(1, parts.size()); - SharedStringsTable stbl = new SharedStringsTable(parts.get(0)); - ReadOnlySharedStringsTable rtbl = new ReadOnlySharedStringsTable(parts.get(0)); - ReadOnlySharedStringsTable rtbl2; - try (InputStream stream = parts.get(0).getInputStream()){ - rtbl2 = new ReadOnlySharedStringsTable(stream); - } - - assertEquals(stbl.getCount(), rtbl.getCount()); - assertEquals(stbl.getUniqueCount(), rtbl.getUniqueCount()); - assertEquals(stbl.getUniqueCount(), rtbl2.getUniqueCount()); - - assertEquals(stbl.getCount(), stbl.getUniqueCount()); - assertEquals(rtbl.getCount(), rtbl.getUniqueCount()); - assertEquals(rtbl.getCount(), rtbl2.getUniqueCount()); - for (int i = 0; i < stbl.getUniqueCount(); i++) { - RichTextString i1 = stbl.getItemAt(i); - assertEquals(i1.getString(), rtbl.getItemAt(i).getString()); - assertEquals(i1.getString(), rtbl2.getItemAt(i).getString()); + try (SharedStringsTable stbl = new SharedStringsTable(parts.get(0))) { + ReadOnlySharedStringsTable rtbl = new ReadOnlySharedStringsTable(parts.get(0)); + ReadOnlySharedStringsTable rtbl2; + try (InputStream stream = parts.get(0).getInputStream()) { + rtbl2 = new ReadOnlySharedStringsTable(stream); + } + + assertEquals(stbl.getCount(), rtbl.getCount()); + assertEquals(stbl.getUniqueCount(), rtbl.getUniqueCount()); + assertEquals(stbl.getUniqueCount(), rtbl2.getUniqueCount()); + + assertEquals(stbl.getCount(), stbl.getUniqueCount()); + assertEquals(rtbl.getCount(), rtbl.getUniqueCount()); + assertEquals(rtbl.getCount(), rtbl2.getUniqueCount()); + for (int i = 0; i < stbl.getUniqueCount(); i++) { + RichTextString i1 = stbl.getItemAt(i); + assertEquals(i1.getString(), rtbl.getItemAt(i).getString()); + assertEquals(i1.getString(), rtbl2.getItemAt(i).getString()); + } } } } @@ -75,20 +77,21 @@ public final class TestReadOnlySharedStringsTable { List<PackagePart> parts = pkg.getPartsByName(Pattern.compile("/xl/sharedStrings.xml")); assertEquals(1, parts.size()); - SharedStringsTable stbl = new SharedStringsTable(parts.get(0)); - ReadOnlySharedStringsTable rtbl = new ReadOnlySharedStringsTable(parts.get(0)); - ReadOnlySharedStringsTable rtbl2; - try (InputStream stream = parts.get(0).getInputStream()) { - rtbl2 = new ReadOnlySharedStringsTable(stream); - } - - assertEquals(stbl.getCount(), rtbl.getCount()); - assertEquals(stbl.getUniqueCount(), rtbl.getUniqueCount()); - assertEquals(stbl.getUniqueCount(), rtbl2.getUniqueCount()); - for (int i = 0; i < stbl.getUniqueCount(); i++) { - RichTextString i1 = stbl.getItemAt(i); - assertEquals(i1.getString(), rtbl.getItemAt(i).getString()); - assertEquals(i1.getString(), rtbl2.getItemAt(i).getString()); + try (SharedStringsTable stbl = new SharedStringsTable(parts.get(0))) { + ReadOnlySharedStringsTable rtbl = new ReadOnlySharedStringsTable(parts.get(0)); + ReadOnlySharedStringsTable rtbl2; + try (InputStream stream = parts.get(0).getInputStream()) { + rtbl2 = new ReadOnlySharedStringsTable(stream); + } + + assertEquals(stbl.getCount(), rtbl.getCount()); + assertEquals(stbl.getUniqueCount(), rtbl.getUniqueCount()); + assertEquals(stbl.getUniqueCount(), rtbl2.getUniqueCount()); + for (int i = 0; i < stbl.getUniqueCount(); i++) { + RichTextString i1 = stbl.getItemAt(i); + assertEquals(i1.getString(), rtbl.getItemAt(i).getString()); + assertEquals(i1.getString(), rtbl2.getItemAt(i).getString()); + } } } } @@ -130,6 +133,22 @@ public final class TestReadOnlySharedStringsTable { } } + @Test + void testNullPointerException() throws Exception { + try (OPCPackage pkg = OPCPackage.open(_ssTests.openResourceAsStream("clusterfuzz-testcase-minimized-XLSX2CSVFuzzer-5025401116950528.xlsx"))) { + assertEmptySST(pkg); + } + + try (OPCPackage pkg = OPCPackage.open(_ssTests.openResourceAsStream("clusterfuzz-testcase-minimized-XLSX2CSVFuzzer-5025401116950528.xlsx"))) { + List<PackagePart> parts = pkg.getPartsByName(Pattern.compile("/xl/sharedStrings.xml")); + assertEquals(1, parts.size()); + + //noinspection resource + assertThrows(IOException.class, + () -> new SharedStringsTable(parts.get(0))); + } + } + private void assertEmptySST(OPCPackage pkg) throws IOException, SAXException { ReadOnlySharedStringsTable sst = new ReadOnlySharedStringsTable(pkg); assertEquals(0, sst.getCount()); diff --git a/test-data/spreadsheet/clusterfuzz-testcase-minimized-XLSX2CSVFuzzer-5025401116950528.xlsx b/test-data/spreadsheet/clusterfuzz-testcase-minimized-XLSX2CSVFuzzer-5025401116950528.xlsx Binary files differnew file mode 100644 index 0000000000..391aa7b2a8 --- /dev/null +++ b/test-data/spreadsheet/clusterfuzz-testcase-minimized-XLSX2CSVFuzzer-5025401116950528.xlsx diff --git a/test-data/spreadsheet/stress.xls b/test-data/spreadsheet/stress.xls Binary files differindex 3215a910a3..a873b632cb 100644 --- a/test-data/spreadsheet/stress.xls +++ b/test-data/spreadsheet/stress.xls |