From 7b15aefae8222b2e61e4d52cddcdfdfff4e29a2b Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Tue, 16 Jul 2024 13:26:16 +0000 Subject: [PATCH] Bug 66425: Avoid exceptions found via poi-fuzz Avoid a possible OutOfMemoryException with incorrect uniqueCount The ReadOnlySharedStringsTable pre-allocates whatever is stated in uniqueCount. As the uniqueCount may be an incorrect large number, we should cap it at some point to avoid OOMs if corrupt files are processed. Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=66137 git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1919284 13f79535-47bb-0310-9956-ffa450edef68 --- .../ReadOnlySharedStringsTable.java | 5 ++- .../TestReadOnlySharedStringsTable.java | 39 +++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/poi-ooxml/src/main/java/org/apache/poi/xssf/eventusermodel/ReadOnlySharedStringsTable.java b/poi-ooxml/src/main/java/org/apache/poi/xssf/eventusermodel/ReadOnlySharedStringsTable.java index 590251960a..bbdd30915c 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/xssf/eventusermodel/ReadOnlySharedStringsTable.java +++ b/poi-ooxml/src/main/java/org/apache/poi/xssf/eventusermodel/ReadOnlySharedStringsTable.java @@ -248,7 +248,10 @@ public class ReadOnlySharedStringsTable extends DefaultHandler implements Shared String uniqueCount = attributes.getValue("uniqueCount"); if(uniqueCount != null) this.uniqueCount = (int) Long.parseLong(uniqueCount); - this.strings = new ArrayList<>(this.uniqueCount); + this.strings = new ArrayList<>( + // corrupted files may have a very large number here, so only use it + // up to some size as guideline for pre-allocating the list + Math.min(this.uniqueCount, 100_000)); characters = new StringBuilder(64); } else if ("si".equals(localName)) { if (characters != null) { diff --git a/poi-ooxml/src/test/java/org/apache/poi/xssf/eventusermodel/TestReadOnlySharedStringsTable.java b/poi-ooxml/src/test/java/org/apache/poi/xssf/eventusermodel/TestReadOnlySharedStringsTable.java index 220963db79..4c7797695f 100644 --- a/poi-ooxml/src/test/java/org/apache/poi/xssf/eventusermodel/TestReadOnlySharedStringsTable.java +++ b/poi-ooxml/src/test/java/org/apache/poi/xssf/eventusermodel/TestReadOnlySharedStringsTable.java @@ -20,10 +20,14 @@ package org.apache.poi.xssf.eventusermodel; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; +import java.nio.charset.StandardCharsets; import java.util.List; import java.util.regex.Pattern; @@ -160,4 +164,39 @@ public final class TestReadOnlySharedStringsTable { assertEquals(0, sst.getCount()); assertEquals(0, sst.getUniqueCount()); } + + private static final String MINIMAL_XML = "" + + "" + + "" + + "bla" + + "" + + "" + + ""; + + @Test + void testMinimalTable() throws IOException, SAXException { + ReadOnlySharedStringsTable tbl = new ReadOnlySharedStringsTable( + new ByteArrayInputStream(MINIMAL_XML.getBytes(StandardCharsets.UTF_8))); + assertNotNull(tbl); + assertEquals(49, tbl.getUniqueCount()); + assertEquals(55, tbl.getCount()); + assertTrue(tbl.includePhoneticRuns); + assertEquals("bla", tbl.getItemAt(0).getString()); + assertThrows(IllegalStateException.class, + () -> tbl.getItemAt(1).getString()); + } + + @Test + void testHugeUniqueCount() throws IOException, SAXException { + ReadOnlySharedStringsTable tbl = new ReadOnlySharedStringsTable( + new ByteArrayInputStream(MINIMAL_XML.replace("49", "99999999999999999"). + getBytes(StandardCharsets.UTF_8))); + assertNotNull(tbl); + assertEquals(1569325055, tbl.getUniqueCount()); + assertEquals(55, tbl.getCount()); + assertTrue(tbl.includePhoneticRuns); + assertEquals("bla", tbl.getItemAt(0).getString()); + assertThrows(IllegalStateException.class, + () -> tbl.getItemAt(1).getString()); + } } -- 2.39.5