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

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 b172b38295a3f99edcee127c2afdb46026292d11..0867f12b81ab17f979827cea60d210700811e56d 100644 (file)
--- 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;
@@ -94,6 +96,25 @@ public class TestXLSX2CSV {
         assertTrue(output.contains(",\"hello, xssf\",,\"hello, xssf\""), "Had: " + output);
     }
 
+    @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();

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 446375affdd8a496a358a857b3b451dadc4b824d..a9f3b37aee69489f51f96cd6dccff27a22bea314 100644 (file)
--- 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 6336836821984dc643ebe36f90d8d96e39587729..07d00bd99aa8ae5382adcbb9496c12b26aa91661 100644 (file)
--- 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 7df35d713c9fd1b8726760b8815ad2827e924246..be486270ed424bf4239942141cbe29008a6bde23 100644 (file)
--- 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
new file mode 100644 (file)
index 0000000..391aa7b
Binary files /dev/null and b/test-data/spreadsheet/clusterfuzz-testcase-minimized-XLSX2CSVFuzzer-5025401116950528.xlsx differ

diff --git a/test-data/spreadsheet/stress.xls b/test-data/spreadsheet/stress.xls
index 3215a910a349916eca0db9d36fabccd5f14350d9..a873b632cb72e537a614246635bd3f62aa38e8ef 100644 (file)
Binary files a/test-data/spreadsheet/stress.xls and b/test-data/spreadsheet/stress.xls differ