Bug 66425: Avoid exceptions found via poi-fuzz

Avoid a possible StackOverflowException

This adds support of counting of the "nesting level" into the base
EscherRecord and thus makes this existing limitation much more effective
as it kicks in for more types of nested records.

Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=66374

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

diff --git a/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/BaseTestPPTIterating.java b/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/BaseTestPPTIterating.java
index 79b200dcca7d7ae18b48d751003cd52586a255fb..5372a23bbf4a793c49dc33418f2012cf3a14d3c1 100644 (file)
--- a/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/BaseTestPPTIterating.java
+++ b/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/BaseTestPPTIterating.java
@@ -71,6 +71,7 @@ public abstract class BaseTestPPTIterating {
         EXCLUDED.put("clusterfuzz-testcase-minimized-POIHSLFFuzzer-4838893004128256.ppt", FileNotFoundException.class);
         EXCLUDED.put("clusterfuzz-testcase-minimized-POIHSLFFuzzer-4624961081573376.ppt", FileNotFoundException.class);
         EXCLUDED.put("clusterfuzz-testcase-minimized-POIHSLFFuzzer-5018229722382336.ppt", RuntimeException.class);
+        EXCLUDED.put("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6192650357112832.ppt", RuntimeException.class);
     }
 
     public static Stream<Arguments> files() {

diff --git a/poi/src/main/java/org/apache/poi/ddf/EscherContainerRecord.java b/poi/src/main/java/org/apache/poi/ddf/EscherContainerRecord.java
index ab30e2c404e1017a5b2266a5ac03367107ad9a8b..747dbde1a59cd9f4e58b9b9e6ed4812d1b33a482 100644 (file)
--- a/poi/src/main/java/org/apache/poi/ddf/EscherContainerRecord.java
+++ b/poi/src/main/java/org/apache/poi/ddf/EscherContainerRecord.java
@@ -91,7 +91,8 @@ public final class EscherContainerRecord extends EscherRecord implements Iterabl
         return fillFields(data, pOffset, recordFactory, 0);
     }
 
-    int fillFields(byte[] data, int pOffset, EscherRecordFactory recordFactory, int nesting) {
+    @Override
+    protected int fillFields(byte[] data, int pOffset, EscherRecordFactory recordFactory, int nesting) {
         if (nesting > MAX_NESTED_CHILD_NODES) {
             throw new IllegalStateException("Had more than the limit of " + MAX_NESTED_CHILD_NODES + " nested child notes");
         }
@@ -107,7 +108,7 @@ public final class EscherContainerRecord extends EscherRecord implements Iterabl
             } else if (child instanceof UnknownEscherRecord) {
                 childBytesWritten = ((UnknownEscherRecord)child).fillFields(data, offset, recordFactory, nesting + 1);
             } else {
-                childBytesWritten = child.fillFields(data, offset, recordFactory);
+                childBytesWritten = child.fillFields(data, offset, recordFactory, nesting + 1);
             }
 
             bytesWritten += childBytesWritten;

diff --git a/poi/src/main/java/org/apache/poi/ddf/EscherRecord.java b/poi/src/main/java/org/apache/poi/ddf/EscherRecord.java
index c817e7d06032fc6ab18918b43923e166bf322507..3d9aeacca8554ae496c79d13c5cf8bcc60c344ad 100644 (file)
--- a/poi/src/main/java/org/apache/poi/ddf/EscherRecord.java
+++ b/poi/src/main/java/org/apache/poi/ddf/EscherRecord.java
@@ -83,6 +83,31 @@ public abstract class EscherRecord implements Duplicatable, GenericRecord {
      */
     public abstract int fillFields( byte[] data, int offset, EscherRecordFactory recordFactory );
 
+    /**
+     * Internal method to prevent too deep nesting/using too much memory.
+     *
+     * This is done by counting the level of "nesting" via the parameter.
+     *
+     * The default method just forwards to fillFields() so it does not properly
+     * handle nesting. Subclasses which do recursive calls need to pass
+     * around the nesting-level properly.
+     *
+     * Usually both fillFields() methods should be overwritten by subclasses,
+     * the one without the "nesting"-parameter should routes to this one in
+     * classes which overwrite this method and this method should be overwritten
+     * with the actual functionality to fill fields.
+     *
+     * @param data      The byte array containing the serialized escher
+     *                  records.
+     * @param offset    The offset into the byte array.
+     * @param recordFactory     A factory for creating new escher records.
+     * @param nesting   The current nesting factor, usually increased by one on each recursive call
+     * @return          The number of bytes written.
+     */
+    protected int fillFields(byte[] data, int offset, EscherRecordFactory recordFactory, int nesting) {
+        return fillFields(data, offset, recordFactory);
+    }
+
     /**
      * Reads the 8 byte header information and populates the <code>options</code>
      * and <code>recordId</code> records.

diff --git a/poi/src/main/java/org/apache/poi/ddf/UnknownEscherRecord.java b/poi/src/main/java/org/apache/poi/ddf/UnknownEscherRecord.java
index 475cfdecaef1a5aa1a59c9d62ad715bf7ca6c60e..a81286c5c72af4f053aca359678fcf22a837a38f 100644 (file)
--- a/poi/src/main/java/org/apache/poi/ddf/UnknownEscherRecord.java
+++ b/poi/src/main/java/org/apache/poi/ddf/UnknownEscherRecord.java
@@ -70,7 +70,8 @@ public final class UnknownEscherRecord extends EscherRecord {
         return fillFields(data, offset, recordFactory, 0);
     }
 
-    int fillFields(byte[] data, int offset, EscherRecordFactory recordFactory, int nesting) {
+    @Override
+    protected int fillFields(byte[] data, int offset, EscherRecordFactory recordFactory, int nesting) {
         if (nesting > MAX_NESTED_CHILD_NODES) {
             throw new IllegalStateException("Had more than the limit of " + MAX_NESTED_CHILD_NODES + " nested child notes");
         }
@@ -97,7 +98,7 @@ public final class UnknownEscherRecord extends EscherRecord {
                 if (child instanceof EscherContainerRecord) {
                     childBytesWritten = ((EscherContainerRecord)child).fillFields(data, offset, recordFactory, nesting + 1);
                 } else {
-                    childBytesWritten = child.fillFields(data, offset, recordFactory);
+                    childBytesWritten = child.fillFields(data, offset, recordFactory, nesting + 1);
                 }
                 bytesWritten += childBytesWritten;
                 offset += childBytesWritten;

diff --git a/test-data/slideshow/clusterfuzz-testcase-minimized-POIHSLFFuzzer-6192650357112832.ppt b/test-data/slideshow/clusterfuzz-testcase-minimized-POIHSLFFuzzer-6192650357112832.ppt
new file mode 100644 (file)
index 0000000..52aab94
Binary files /dev/null and b/test-data/slideshow/clusterfuzz-testcase-minimized-POIHSLFFuzzer-6192650357112832.ppt differ

diff --git a/test-data/spreadsheet/stress.xls b/test-data/spreadsheet/stress.xls
index a94383250f1d68063daf015276983f5feddf8525..78bed83bc03e28c6a52effe35ceddc2f839a0333 100644 (file)
Binary files a/test-data/spreadsheet/stress.xls and b/test-data/spreadsheet/stress.xls differ