From: Dominik Stadler Date: Mon, 15 Jul 2024 13:02:43 +0000 (+0000) Subject: Bug 66425: Avoid exceptions found via poi-fuzz X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=2582e5e0c18026c88b882829254241a9acc6f45f;p=poi.git 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 79b200dcca..5372a23bbf 100644 --- 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 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 ab30e2c404..747dbde1a5 100644 --- 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 c817e7d060..3d9aeacca8 100644 --- 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 options * and recordId 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 475cfdecae..a81286c5c7 100644 --- 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 index 0000000000..52aab948e4 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 a94383250f..78bed83bc0 100644 Binary files a/test-data/spreadsheet/stress.xls and b/test-data/spreadsheet/stress.xls differ