mirror of https://github.com/apache/poi.git
Bug 66425: Avoid exceptions found via poi-fuzz
Avoid a possible OutOfMemoryException with many child-records This avoids having too many children in EscherRecords, the limit of 100_000 is arbitrarily chosen and can be adjusted if needed Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=62924 and maybe others git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1919272 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
parent
9456261cba
commit
09fbfd5be4
|
@ -72,6 +72,7 @@ public abstract class BaseTestPPTIterating {
|
|||
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);
|
||||
EXCLUDED.put("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6614960949821440.ppt", RuntimeException.class);
|
||||
}
|
||||
|
||||
public static Stream<Arguments> files() {
|
||||
|
|
|
@ -36,6 +36,7 @@ public class TestPPTXMLDump extends BaseTestPPTIterating {
|
|||
LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6032591399288832.ppt");
|
||||
LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6360479850954752.ppt");
|
||||
LOCAL_EXCLUDED.add("ppt_with_png_encrypted.ppt");
|
||||
LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6614960949821440.ppt");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
|
@ -41,6 +41,11 @@ public class TestSlideShowDumper extends BaseTestPPTIterating {
|
|||
FAILING.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6360479850954752.ppt");
|
||||
}
|
||||
|
||||
static final Set<String> LOCAL_EXCLUDED = new HashSet<>();
|
||||
static {
|
||||
LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6614960949821440.ppt");
|
||||
}
|
||||
|
||||
@Test
|
||||
void testMain() throws IOException {
|
||||
// SlideShowDumper calls IOUtils.toByteArray(is), which would fail if a different size is defined
|
||||
|
@ -71,6 +76,11 @@ public class TestSlideShowDumper extends BaseTestPPTIterating {
|
|||
throw e;
|
||||
}
|
||||
}
|
||||
|
||||
// these fail everywhere else, so also should fail here
|
||||
if (LOCAL_EXCLUDED.contains(pFile.getName())) {
|
||||
throw new RuntimeException();
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
|
@ -208,6 +208,12 @@ public final class EscherContainerRecord extends EscherRecord implements Iterabl
|
|||
if (childRecords == _childRecords) {
|
||||
throw new IllegalStateException("Child records private data member has escaped");
|
||||
}
|
||||
|
||||
if (childRecords.size() > MAX_NUMBER_OF_CHILDREN) {
|
||||
throw new IllegalStateException("Cannot add more than " + MAX_NUMBER_OF_CHILDREN +
|
||||
" child records, you can use 'EscherRecord.setMaxNumberOfChildren()' to increase the allow size");
|
||||
}
|
||||
|
||||
_childRecords.clear();
|
||||
_childRecords.addAll(childRecords);
|
||||
}
|
||||
|
@ -261,6 +267,11 @@ public final class EscherContainerRecord extends EscherRecord implements Iterabl
|
|||
* @param record the record to be added
|
||||
*/
|
||||
public void addChildRecord(EscherRecord record) {
|
||||
if (_childRecords.size() >= MAX_NUMBER_OF_CHILDREN) {
|
||||
throw new IllegalStateException("Cannot add more than " + MAX_NUMBER_OF_CHILDREN +
|
||||
" child records, you can use 'EscherRecord.setMaxNumberOfChildren()' to increase the allow size");
|
||||
}
|
||||
|
||||
_childRecords.add(record);
|
||||
}
|
||||
|
||||
|
|
|
@ -45,6 +45,10 @@ public abstract class EscherRecord implements Duplicatable, GenericRecord {
|
|||
private short _options;
|
||||
private short _recordId;
|
||||
|
||||
// arbitrarily selected; may need to increase
|
||||
private static final int DEFAULT_MAX_NUMBER_OF_CHILDREN = 100_000;
|
||||
protected static int MAX_NUMBER_OF_CHILDREN = DEFAULT_MAX_NUMBER_OF_CHILDREN;
|
||||
|
||||
/**
|
||||
* Create a new instance
|
||||
*/
|
||||
|
@ -367,4 +371,18 @@ public abstract class EscherRecord implements Duplicatable, GenericRecord {
|
|||
|
||||
@Override
|
||||
public abstract EscherRecord copy();
|
||||
|
||||
/**
|
||||
* @param length the max record length allowed for EscherArrayProperty
|
||||
*/
|
||||
public static void setMaxNumberOfChildren(int length) {
|
||||
MAX_NUMBER_OF_CHILDREN = length;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return the max record length allowed for EscherArrayProperty
|
||||
*/
|
||||
public static int getMaxNumberOfChildren() {
|
||||
return MAX_NUMBER_OF_CHILDREN;
|
||||
}
|
||||
}
|
|
@ -160,6 +160,12 @@ public final class UnknownEscherRecord extends EscherRecord {
|
|||
if (childRecords == _childRecords) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (childRecords.size() > MAX_NUMBER_OF_CHILDREN) {
|
||||
throw new IllegalStateException("Cannot add more than " + MAX_NUMBER_OF_CHILDREN +
|
||||
" child records, you can use 'EscherRecord.setMaxNumberOfChildren()' to increase the allow size");
|
||||
}
|
||||
|
||||
_childRecords.clear();
|
||||
_childRecords.addAll(childRecords);
|
||||
}
|
||||
|
@ -170,6 +176,11 @@ public final class UnknownEscherRecord extends EscherRecord {
|
|||
}
|
||||
|
||||
public void addChildRecord(EscherRecord childRecord) {
|
||||
if (_childRecords.size() >= MAX_NUMBER_OF_CHILDREN) {
|
||||
throw new IllegalStateException("Cannot add more than " + MAX_NUMBER_OF_CHILDREN +
|
||||
" child records, you can use 'EscherRecord.setMaxNumberOfChildren()' to increase the allow size");
|
||||
}
|
||||
|
||||
getChildRecords().add( childRecord );
|
||||
}
|
||||
|
||||
|
|
|
@ -20,19 +20,23 @@ package org.apache.poi.ddf;
|
|||
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
|
||||
import static org.junit.jupiter.api.Assertions.assertNotSame;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||
|
||||
import java.util.Arrays;
|
||||
import java.util.List;
|
||||
|
||||
import org.apache.poi.POIDataSamples;
|
||||
import org.apache.poi.util.HexDump;
|
||||
import org.apache.poi.util.HexRead;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.parallel.Isolated;
|
||||
|
||||
/**
|
||||
* Tests for {@link EscherContainerRecord}
|
||||
*/
|
||||
@Isolated // this test changes global static MAX_NUMBER_OF_CHILDREN
|
||||
final class TestEscherContainerRecord {
|
||||
private static final POIDataSamples _samples = POIDataSamples.getDDFInstance();
|
||||
|
||||
|
@ -42,7 +46,7 @@ final class TestEscherContainerRecord {
|
|||
byte[] data = HexRead.readFromString("0F 02 11 F1 00 00 00 00");
|
||||
EscherRecord r = f.createRecord(data, 0);
|
||||
r.fillFields(data, 0, f);
|
||||
assertTrue(r instanceof EscherContainerRecord);
|
||||
assertInstanceOf(EscherContainerRecord.class, r);
|
||||
assertEquals((short) 0x020F, r.getOptions());
|
||||
assertEquals((short) 0xF111, r.getRecordId());
|
||||
|
||||
|
@ -91,7 +95,7 @@ final class TestEscherContainerRecord {
|
|||
"\t, \"recordSize\": 8\n" +
|
||||
"\t, \"isContainer\": true\n" +
|
||||
"}";
|
||||
expected = expected.replace("\n", System.getProperty("line.separator"));
|
||||
expected = expected.replace("\n", System.lineSeparator());
|
||||
assertEquals(expected, r.toString());
|
||||
|
||||
EscherOptRecord r2 = new EscherOptRecord();
|
||||
|
@ -121,7 +125,7 @@ final class TestEscherContainerRecord {
|
|||
"\t\t}\n" +
|
||||
"\t]\n" +
|
||||
"}";
|
||||
expected = expected.replace("\n", System.getProperty("line.separator"));
|
||||
expected = expected.replace("\n", System.lineSeparator());
|
||||
assertEquals(expected, r.toString());
|
||||
|
||||
r.addChildRecord(r2);
|
||||
|
@ -155,7 +159,7 @@ final class TestEscherContainerRecord {
|
|||
"\t\t}\n" +
|
||||
"\t]\n" +
|
||||
"}";
|
||||
expected = expected.replace("\n", System.getProperty("line.separator"));
|
||||
expected = expected.replace("\n", System.lineSeparator());
|
||||
assertEquals(expected, r.toString());
|
||||
}
|
||||
|
||||
|
@ -170,7 +174,7 @@ final class TestEscherContainerRecord {
|
|||
@Override
|
||||
public String getRecordName() { return ""; }
|
||||
@Override
|
||||
public Enum getGenericRecordType() { return EscherRecordTypes.UNKNOWN; }
|
||||
public Enum<?> getGenericRecordType() { return EscherRecordTypes.UNKNOWN; }
|
||||
@Override
|
||||
public DummyEscherRecord copy() { return null; }
|
||||
}
|
||||
|
@ -231,4 +235,25 @@ final class TestEscherContainerRecord {
|
|||
assertEquals(chC, children2.get(0));
|
||||
assertEquals(chA, children2.get(1));
|
||||
}
|
||||
|
||||
@Test
|
||||
void testTooManyChildren() {
|
||||
EscherContainerRecord ecr = new EscherContainerRecord();
|
||||
|
||||
int before = EscherRecord.getMaxNumberOfChildren();
|
||||
try {
|
||||
EscherRecord.setMaxNumberOfChildren(2);
|
||||
|
||||
ecr.addChildRecord(new EscherDgRecord());
|
||||
ecr.addChildRecord(new EscherDgRecord());
|
||||
assertThrows(IllegalStateException.class,
|
||||
() -> ecr.addChildRecord(new EscherDgRecord()));
|
||||
|
||||
ecr.setChildRecords(Arrays.asList(new EscherDgRecord(), new EscherDgRecord()));
|
||||
assertThrows(IllegalStateException.class,
|
||||
() -> ecr.setChildRecords(Arrays.asList(new EscherDgRecord(), new EscherDgRecord(), new EscherDgRecord())));
|
||||
} finally {
|
||||
EscherRecord.setMaxNumberOfChildren(before);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -19,12 +19,17 @@ package org.apache.poi.ddf;
|
|||
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
import org.apache.poi.util.HexDump;
|
||||
import org.apache.poi.util.HexRead;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.parallel.Isolated;
|
||||
|
||||
import java.util.Arrays;
|
||||
|
||||
@Isolated // this test changes global static MAX_NUMBER_OF_CHILDREN
|
||||
final class TestUnknownEscherRecord {
|
||||
@Test
|
||||
void testFillFields() {
|
||||
|
@ -159,7 +164,28 @@ final class TestUnknownEscherRecord {
|
|||
"\t, \"recordSize\": 8\n" +
|
||||
"\t, \"data\": \"\"\n" +
|
||||
"}";
|
||||
expected = expected.replace("\n", System.getProperty("line.separator"));
|
||||
expected = expected.replace("\n", System.lineSeparator());
|
||||
assertEquals(expected, r.toString() );
|
||||
}
|
||||
|
||||
@Test
|
||||
void testTooManyChildren() {
|
||||
UnknownEscherRecord ecr = new UnknownEscherRecord();
|
||||
|
||||
int before = EscherRecord.getMaxNumberOfChildren();
|
||||
try {
|
||||
EscherRecord.setMaxNumberOfChildren(2);
|
||||
|
||||
ecr.addChildRecord(new EscherDgRecord());
|
||||
ecr.addChildRecord(new EscherDgRecord());
|
||||
assertThrows(IllegalStateException.class,
|
||||
() -> ecr.addChildRecord(new EscherDgRecord()));
|
||||
|
||||
ecr.setChildRecords(Arrays.asList(new EscherDgRecord(), new EscherDgRecord()));
|
||||
assertThrows(IllegalStateException.class,
|
||||
() -> ecr.setChildRecords(Arrays.asList(new EscherDgRecord(), new EscherDgRecord(), new EscherDgRecord())));
|
||||
} finally {
|
||||
EscherRecord.setMaxNumberOfChildren(before);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
Binary file not shown.
Binary file not shown.
Loading…
Reference in New Issue