Author: centic Date: Tue Jul 16 05:26:42 2024 New Revision: 1919272 URL: http://svn.apache.org/viewvc?rev=1919272&view=rev Log: 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 Added: poi/trunk/test-data/slideshow/clusterfuzz-testcase-minimized-POIHSLFFuzzer-6614960949821440.ppt (with props) Modified: poi/trunk/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/BaseTestPPTIterating.java poi/trunk/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestPPTXMLDump.java poi/trunk/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideShowDumper.java poi/trunk/poi/src/main/java/org/apache/poi/ddf/EscherContainerRecord.java poi/trunk/poi/src/main/java/org/apache/poi/ddf/EscherRecord.java poi/trunk/poi/src/main/java/org/apache/poi/ddf/UnknownEscherRecord.java poi/trunk/poi/src/test/java/org/apache/poi/ddf/TestEscherContainerRecord.java poi/trunk/poi/src/test/java/org/apache/poi/ddf/TestUnknownEscherRecord.java poi/trunk/test-data/spreadsheet/stress.xls Modified: poi/trunk/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/BaseTestPPTIterating.java URL: http://svn.apache.org/viewvc/poi/trunk/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/BaseTestPPTIterating.java?rev=1919272&r1=1919271&r2=1919272&view=diff ============================================================================== --- poi/trunk/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/BaseTestPPTIterating.java (original) +++ poi/trunk/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/BaseTestPPTIterating.java Tue Jul 16 05:26:42 2024 @@ -72,6 +72,7 @@ public abstract class BaseTestPPTIterati 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() { Modified: poi/trunk/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestPPTXMLDump.java URL: http://svn.apache.org/viewvc/poi/trunk/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestPPTXMLDump.java?rev=1919272&r1=1919271&r2=1919272&view=diff ============================================================================== --- poi/trunk/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestPPTXMLDump.java (original) +++ poi/trunk/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestPPTXMLDump.java Tue Jul 16 05:26:42 2024 @@ -36,6 +36,7 @@ public class TestPPTXMLDump extends Base 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 Modified: poi/trunk/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideShowDumper.java URL: http://svn.apache.org/viewvc/poi/trunk/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideShowDumper.java?rev=1919272&r1=1919271&r2=1919272&view=diff ============================================================================== --- poi/trunk/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideShowDumper.java (original) +++ poi/trunk/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideShowDumper.java Tue Jul 16 05:26:42 2024 @@ -41,6 +41,11 @@ public class TestSlideShowDumper extends 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 throw e; } } + + // these fail everywhere else, so also should fail here + if (LOCAL_EXCLUDED.contains(pFile.getName())) { + throw new RuntimeException(); + } } @Override Modified: poi/trunk/poi/src/main/java/org/apache/poi/ddf/EscherContainerRecord.java URL: http://svn.apache.org/viewvc/poi/trunk/poi/src/main/java/org/apache/poi/ddf/EscherContainerRecord.java?rev=1919272&r1=1919271&r2=1919272&view=diff ============================================================================== --- poi/trunk/poi/src/main/java/org/apache/poi/ddf/EscherContainerRecord.java (original) +++ poi/trunk/poi/src/main/java/org/apache/poi/ddf/EscherContainerRecord.java Tue Jul 16 05:26:42 2024 @@ -208,6 +208,12 @@ public final class EscherContainerRecord 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 * @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); } Modified: poi/trunk/poi/src/main/java/org/apache/poi/ddf/EscherRecord.java URL: http://svn.apache.org/viewvc/poi/trunk/poi/src/main/java/org/apache/poi/ddf/EscherRecord.java?rev=1919272&r1=1919271&r2=1919272&view=diff ============================================================================== --- poi/trunk/poi/src/main/java/org/apache/poi/ddf/EscherRecord.java (original) +++ poi/trunk/poi/src/main/java/org/apache/poi/ddf/EscherRecord.java Tue Jul 16 05:26:42 2024 @@ -45,6 +45,10 @@ public abstract class EscherRecord imple 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 imple @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; + } } \ No newline at end of file Modified: poi/trunk/poi/src/main/java/org/apache/poi/ddf/UnknownEscherRecord.java URL: http://svn.apache.org/viewvc/poi/trunk/poi/src/main/java/org/apache/poi/ddf/UnknownEscherRecord.java?rev=1919272&r1=1919271&r2=1919272&view=diff ============================================================================== --- poi/trunk/poi/src/main/java/org/apache/poi/ddf/UnknownEscherRecord.java (original) +++ poi/trunk/poi/src/main/java/org/apache/poi/ddf/UnknownEscherRecord.java Tue Jul 16 05:26:42 2024 @@ -160,6 +160,12 @@ public final class UnknownEscherRecord e 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 e } 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 ); } Modified: poi/trunk/poi/src/test/java/org/apache/poi/ddf/TestEscherContainerRecord.java URL: http://svn.apache.org/viewvc/poi/trunk/poi/src/test/java/org/apache/poi/ddf/TestEscherContainerRecord.java?rev=1919272&r1=1919271&r2=1919272&view=diff ============================================================================== --- poi/trunk/poi/src/test/java/org/apache/poi/ddf/TestEscherContainerRecord.java (original) +++ poi/trunk/poi/src/test/java/org/apache/poi/ddf/TestEscherContainerRecord.java Tue Jul 16 05:26:42 2024 @@ -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); + } + } } Modified: poi/trunk/poi/src/test/java/org/apache/poi/ddf/TestUnknownEscherRecord.java URL: http://svn.apache.org/viewvc/poi/trunk/poi/src/test/java/org/apache/poi/ddf/TestUnknownEscherRecord.java?rev=1919272&r1=1919271&r2=1919272&view=diff ============================================================================== --- poi/trunk/poi/src/test/java/org/apache/poi/ddf/TestUnknownEscherRecord.java (original) +++ poi/trunk/poi/src/test/java/org/apache/poi/ddf/TestUnknownEscherRecord.java Tue Jul 16 05:26:42 2024 @@ -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); + } + } } Added: poi/trunk/test-data/slideshow/clusterfuzz-testcase-minimized-POIHSLFFuzzer-6614960949821440.ppt URL: http://svn.apache.org/viewvc/poi/trunk/test-data/slideshow/clusterfuzz-testcase-minimized-POIHSLFFuzzer-6614960949821440.ppt?rev=1919272&view=auto ============================================================================== Binary file - no diff available. Propchange: poi/trunk/test-data/slideshow/clusterfuzz-testcase-minimized-POIHSLFFuzzer-6614960949821440.ppt ------------------------------------------------------------------------------ svn:mime-type = application/vnd.ms-powerpoint Modified: poi/trunk/test-data/spreadsheet/stress.xls URL: http://svn.apache.org/viewvc/poi/trunk/test-data/spreadsheet/stress.xls?rev=1919272&r1=1919271&r2=1919272&view=diff ============================================================================== Binary files - no diff available. --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
