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
This commit is contained in:
Dominik Stadler 2024-07-15 13:02:43 +00:00
parent 719e7154a1
commit 2582e5e0c1
6 changed files with 32 additions and 4 deletions

View File

@ -71,6 +71,7 @@ public abstract class BaseTestPPTIterating {
EXCLUDED.put("clusterfuzz-testcase-minimized-POIHSLFFuzzer-4838893004128256.ppt", FileNotFoundException.class); 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-4624961081573376.ppt", FileNotFoundException.class);
EXCLUDED.put("clusterfuzz-testcase-minimized-POIHSLFFuzzer-5018229722382336.ppt", RuntimeException.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() { public static Stream<Arguments> files() {

View File

@ -91,7 +91,8 @@ public final class EscherContainerRecord extends EscherRecord implements Iterabl
return fillFields(data, pOffset, recordFactory, 0); 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) { if (nesting > MAX_NESTED_CHILD_NODES) {
throw new IllegalStateException("Had more than the limit of " + MAX_NESTED_CHILD_NODES + " nested child notes"); 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) { } else if (child instanceof UnknownEscherRecord) {
childBytesWritten = ((UnknownEscherRecord)child).fillFields(data, offset, recordFactory, nesting + 1); childBytesWritten = ((UnknownEscherRecord)child).fillFields(data, offset, recordFactory, nesting + 1);
} else { } else {
childBytesWritten = child.fillFields(data, offset, recordFactory); childBytesWritten = child.fillFields(data, offset, recordFactory, nesting + 1);
} }
bytesWritten += childBytesWritten; bytesWritten += childBytesWritten;

View File

@ -83,6 +83,31 @@ public abstract class EscherRecord implements Duplicatable, GenericRecord {
*/ */
public abstract int fillFields( byte[] data, int offset, EscherRecordFactory recordFactory ); 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> * Reads the 8 byte header information and populates the <code>options</code>
* and <code>recordId</code> records. * and <code>recordId</code> records.

View File

@ -70,7 +70,8 @@ public final class UnknownEscherRecord extends EscherRecord {
return fillFields(data, offset, recordFactory, 0); 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) { if (nesting > MAX_NESTED_CHILD_NODES) {
throw new IllegalStateException("Had more than the limit of " + MAX_NESTED_CHILD_NODES + " nested child notes"); 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) { if (child instanceof EscherContainerRecord) {
childBytesWritten = ((EscherContainerRecord)child).fillFields(data, offset, recordFactory, nesting + 1); childBytesWritten = ((EscherContainerRecord)child).fillFields(data, offset, recordFactory, nesting + 1);
} else { } else {
childBytesWritten = child.fillFields(data, offset, recordFactory); childBytesWritten = child.fillFields(data, offset, recordFactory, nesting + 1);
} }
bytesWritten += childBytesWritten; bytesWritten += childBytesWritten;
offset += childBytesWritten; offset += childBytesWritten;

Binary file not shown.