Bug 66425: Avoid a ClassCastException found via oss-fuzz

We try to avoid throwing ClassCastException, 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=61242

Also enhance output of some test-failures and allow an empty exception message

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

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 a48d0cbc2985902d8b5d98059425841477db4d81..ed96cde4e895718a0fbd841978cbdac4f2ce0fa5 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
@@ -42,6 +42,7 @@ import org.junit.jupiter.api.parallel.ExecutionMode;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
+import org.junit.platform.commons.util.StringUtils;
 import org.opentest4j.AssertionFailedError;
 
 /**
@@ -251,12 +252,12 @@ public class TestAllFiles {
         } else if (exClass != null) {
             Exception e = assertThrows((Class<? extends Exception>)exClass, exec, errPrefix + " expected " + exClass);
             String actMsg = pathReplace(e.getMessage());
-            if (NullPointerException.class.isAssignableFrom(exClass)) {
-                if (actMsg != null) {
-                    assertTrue(actMsg.contains(exMessage), errPrefix + "Message: "+actMsg+" - didn't contain: "+exMessage);
-                }
-            } else {
-                assertNotNull(actMsg, errPrefix);
+
+            // verify that message is either null for both or set for both
+            assertTrue(actMsg != null || StringUtils.isBlank(exMessage),
+                    errPrefix + " for " + exClass + " expected message '" + exMessage + "' but had '" + actMsg + "'");
+
+            if (actMsg != null) {
                 assertTrue(actMsg.contains(exMessage),
                         errPrefix + "Message: " + actMsg + " - didn't contain: " + exMessage);
             }

diff --git a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFShapeGroup.java b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFShapeGroup.java
index ab7df1d733f5963325ce01860799ce9211c49260..1a0fca32e8ea74ff28963c8cb4d69bf59ee36e62 100644 (file)
--- a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFShapeGroup.java
+++ b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFShapeGroup.java
@@ -54,7 +54,11 @@ public class HSSFShapeGroup extends HSSFShape implements HSSFShapeContainer {
 
         // read internal and external coordinates from spgrContainer
         EscherContainerRecord spContainer = spgrContainer.getChildContainers().get(0);
-        _spgrRecord = (EscherSpgrRecord) spContainer.getChild(0);
+        final EscherRecord child = spContainer.getChild(0);
+        if (!(child instanceof EscherSpgrRecord)) {
+            throw new IllegalArgumentException("Had unexpected type of child at index 0: " + child.getClass());
+        }
+        _spgrRecord = (EscherSpgrRecord) child;
         for (EscherRecord ch : spContainer) {
             switch (EscherRecordTypes.forTypeID(ch.getRecordId())) {
                 case CLIENT_ANCHOR:

diff --git a/poi/src/test/java/org/apache/poi/hssf/dev/BaseTestIteratingXLS.java b/poi/src/test/java/org/apache/poi/hssf/dev/BaseTestIteratingXLS.java
index 5ec8a30b23ed7cbc79fc6ad7c6798bbe9c6da435..45de3c21555a2a81eb0385d67acbeb803b76bb8b 100644 (file)
--- a/poi/src/test/java/org/apache/poi/hssf/dev/BaseTestIteratingXLS.java
+++ b/poi/src/test/java/org/apache/poi/hssf/dev/BaseTestIteratingXLS.java
@@ -102,9 +102,9 @@ public abstract class BaseTestIteratingXLS {
 
         Executable ex = () -> runOneFile(file);
         if (t == null) {
-            assertDoesNotThrow(ex);
+            assertDoesNotThrow(ex, "Failing file: " + file);
         } else {
-            assertThrows(t, ex);
+            assertThrows(t, ex, "Failing file: " + file);
         }
     }
 

diff --git a/poi/src/test/java/org/apache/poi/hssf/dev/TestBiffDrawingToXml.java b/poi/src/test/java/org/apache/poi/hssf/dev/TestBiffDrawingToXml.java
index 0b788309961845042678a116c746df98164c308e..ea39f5147dedc9c2607783da90944355c34b7697 100644 (file)
--- a/poi/src/test/java/org/apache/poi/hssf/dev/TestBiffDrawingToXml.java
+++ b/poi/src/test/java/org/apache/poi/hssf/dev/TestBiffDrawingToXml.java
@@ -54,6 +54,7 @@ class TestBiffDrawingToXml extends BaseTestIteratingXLS {
         excludes.put("43493.xls", RecordInputStream.LeftoverDataException.class);
         excludes.put("44958_1.xls", RecordInputStream.LeftoverDataException.class);
         excludes.put("protected_66115.xls", EncryptedDocumentException.class);
+        excludes.put("clusterfuzz-testcase-minimized-POIHSSFFuzzer-5285517825277952.xls", IllegalArgumentException.class);
         return excludes;
     }
 

diff --git a/poi/src/test/java/org/apache/poi/hssf/model/TestDrawingAggregate.java b/poi/src/test/java/org/apache/poi/hssf/model/TestDrawingAggregate.java
index b904af880869d03f5b8d2572f0aadc8d47bf1739..9a3bd3d64955f7432cd0d7c011b18dc88d713320 100644 (file)
--- a/poi/src/test/java/org/apache/poi/hssf/model/TestDrawingAggregate.java
+++ b/poi/src/test/java/org/apache/poi/hssf/model/TestDrawingAggregate.java
@@ -136,7 +136,9 @@ class TestDrawingAggregate {
 
         File[] files = testData.listFiles((dir, name) -> name.endsWith(".xls"));
         assertNotNull(files, "Need to find files in test-data path, had path: " + testData);
-        return Stream.of(files).map(Arguments::of);
+        return Stream.of(files).
+                filter(file -> !file.getName().equals("clusterfuzz-testcase-minimized-POIHSSFFuzzer-5285517825277952.xls")).
+                map(Arguments::of);
     }
 
     /**

diff --git a/test-data/spreadsheet/clusterfuzz-testcase-minimized-POIHSSFFuzzer-5285517825277952.xls b/test-data/spreadsheet/clusterfuzz-testcase-minimized-POIHSSFFuzzer-5285517825277952.xls
new file mode 100644 (file)
index 0000000..544c22f
Binary files /dev/null and b/test-data/spreadsheet/clusterfuzz-testcase-minimized-POIHSSFFuzzer-5285517825277952.xls differ

diff --git a/test-data/spreadsheet/stress.xls b/test-data/spreadsheet/stress.xls
index 301d3e61fc37a2fa0ad653a6c193a8899c0bdd56..3018e322b36706679e4f0e9f7e4e0dcb6f1123bb 100644 (file)
Binary files a/test-data/spreadsheet/stress.xls and b/test-data/spreadsheet/stress.xls differ