From 5cd4fa548843e89525e48d6180ab6f3c5eb9833e Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Sat, 7 Oct 2023 22:12:43 +0000 Subject: Bug 66425: Avoid Exceptions found via oss-fuzz We try to avoid throwing ClassCastExceptions, but it was possible to trigger one here with a specially crafted input-file Should fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=62795 git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1912796 13f79535-47bb-0310-9956-ffa450edef68 --- .../poi/hssf/converter/TestExcelConverterSuite.java | 11 ++++++----- .../org/apache/poi/hssf/model/InternalWorkbook.java | 6 +++++- .../apache/poi/hssf/usermodel/HSSFShapeFactory.java | 6 +++++- .../poi/util/LittleEndianByteArrayInputStream.java | 2 +- .../apache/poi/hssf/dev/TestBiffDrawingToXml.java | 1 + .../apache/poi/hssf/model/TestDrawingAggregate.java | 10 +++++++++- ...ase-minimized-POIHSSFFuzzer-5436547081830400.xls | Bin 0 -> 47616 bytes test-data/spreadsheet/stress.xls | Bin 57344 -> 57856 bytes 8 files changed, 27 insertions(+), 9 deletions(-) create mode 100644 test-data/spreadsheet/clusterfuzz-testcase-minimized-POIHSSFFuzzer-5436547081830400.xls diff --git a/poi-scratchpad/src/test/java/org/apache/poi/hssf/converter/TestExcelConverterSuite.java b/poi-scratchpad/src/test/java/org/apache/poi/hssf/converter/TestExcelConverterSuite.java index 927d54e88e..980e1298b8 100644 --- a/poi-scratchpad/src/test/java/org/apache/poi/hssf/converter/TestExcelConverterSuite.java +++ b/poi-scratchpad/src/test/java/org/apache/poi/hssf/converter/TestExcelConverterSuite.java @@ -38,15 +38,16 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; -public class TestExcelConverterSuite -{ +public class TestExcelConverterSuite { /** * YK: a quick hack to exclude failing documents from the suite. */ - @SuppressWarnings("ArraysAsListWithZeroOrOneArgument") private static final List failingFiles = Arrays.asList( - /* not failing, but requires more memory */ - "ex45698-22488.xls" ); + // not failing, but requires more memory + "ex45698-22488.xls", + // broken documents + "clusterfuzz-testcase-minimized-POIHSSFFuzzer-5436547081830400.xls" + ); public static Stream files() { List files = new ArrayList<>(); diff --git a/poi/src/main/java/org/apache/poi/hssf/model/InternalWorkbook.java b/poi/src/main/java/org/apache/poi/hssf/model/InternalWorkbook.java index acd3541cf7..a60fec9fd6 100644 --- a/poi/src/main/java/org/apache/poi/hssf/model/InternalWorkbook.java +++ b/poi/src/main/java/org/apache/poi/hssf/model/InternalWorkbook.java @@ -466,7 +466,11 @@ public final class InternalWorkbook { "There are only " + numfonts + " font records, but you asked for index " + idx); } - return ( FontRecord ) records.get((records.getFontpos() - (numfonts - 1)) + index); + Record record = records.get((records.getFontpos() - (numfonts - 1)) + index); + if (!(record instanceof FontRecord)) { + throw new IllegalStateException("Did not have the expected record-type FontRecord: " + record.getClass()); + } + return ( FontRecord ) record; } /** diff --git a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFShapeFactory.java b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFShapeFactory.java index fff9250a68..080938367b 100644 --- a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFShapeFactory.java +++ b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFShapeFactory.java @@ -51,7 +51,11 @@ public class HSSFShapeFactory { public static void createShapeTree(EscherContainerRecord container, EscherAggregate agg, HSSFShapeContainer out, DirectoryNode root) { if (container.getRecordId() == EscherContainerRecord.SPGR_CONTAINER) { ObjRecord obj = null; - EscherClientDataRecord clientData = ((EscherContainerRecord) container.getChild(0)).getChildById(EscherClientDataRecord.RECORD_ID); + EscherRecord child = container.getChild(0); + if (!(child instanceof EscherContainerRecord)) { + throw new IllegalArgumentException("Had unexpected type of child: " + child.getClass()); + } + EscherClientDataRecord clientData = ((EscherContainerRecord) child).getChildById(EscherClientDataRecord.RECORD_ID); if (null != clientData) { obj = (ObjRecord) agg.getShapeToObjMapping().get(clientData); } diff --git a/poi/src/main/java/org/apache/poi/util/LittleEndianByteArrayInputStream.java b/poi/src/main/java/org/apache/poi/util/LittleEndianByteArrayInputStream.java index e5c0b259a0..2f7215b0bb 100644 --- a/poi/src/main/java/org/apache/poi/util/LittleEndianByteArrayInputStream.java +++ b/poi/src/main/java/org/apache/poi/util/LittleEndianByteArrayInputStream.java @@ -87,7 +87,7 @@ public class LittleEndianByteArrayInputStream extends ByteArrayInputStream imple public void setReadIndex(int pos) { if (pos < 0 || pos >= count) { - throw new IndexOutOfBoundsException(); + throw new IndexOutOfBoundsException("Invalid position: " + pos + " with count " + count); } this.pos = pos; } diff --git a/poi/src/test/java/org/apache/poi/hssf/dev/TestBiffDrawingToXml.java b/poi/src/test/java/org/apache/poi/hssf/dev/TestBiffDrawingToXml.java index ea39f5147d..a87f34aad8 100644 --- a/poi/src/test/java/org/apache/poi/hssf/dev/TestBiffDrawingToXml.java +++ b/poi/src/test/java/org/apache/poi/hssf/dev/TestBiffDrawingToXml.java @@ -55,6 +55,7 @@ class TestBiffDrawingToXml extends BaseTestIteratingXLS { excludes.put("44958_1.xls", RecordInputStream.LeftoverDataException.class); excludes.put("protected_66115.xls", EncryptedDocumentException.class); excludes.put("clusterfuzz-testcase-minimized-POIHSSFFuzzer-5285517825277952.xls", IllegalArgumentException.class); + excludes.put("clusterfuzz-testcase-minimized-POIHSSFFuzzer-5436547081830400.xls", IllegalArgumentException.class); return excludes; } diff --git a/poi/src/test/java/org/apache/poi/hssf/model/TestDrawingAggregate.java b/poi/src/test/java/org/apache/poi/hssf/model/TestDrawingAggregate.java index 9a3bd3d649..e21e359767 100644 --- a/poi/src/test/java/org/apache/poi/hssf/model/TestDrawingAggregate.java +++ b/poi/src/test/java/org/apache/poi/hssf/model/TestDrawingAggregate.java @@ -21,6 +21,7 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.ByteArrayInputStream; @@ -163,6 +164,12 @@ class TestDrawingAggregate { DrawingAggregateInfo info = DrawingAggregateInfo.get(sheet); if(info != null) { aggs.put(i, info); + if (file.getName().equals("clusterfuzz-testcase-minimized-POIHSSFFuzzer-5436547081830400.xls")) { + assertThrows(IllegalArgumentException.class, + sheet::getDrawingPatriarch); + return; + } + HSSFPatriarch p = sheet.getDrawingPatriarch(); // compare aggregate.serialize() with raw bytes from the record stream @@ -172,7 +179,8 @@ class TestDrawingAggregate { byte[] dgBytes2 = agg.serialize(); assertEquals(dgBytes1.length, dgBytes2.length, "different size of raw data ande aggregate.serialize()"); - assertArrayEquals(dgBytes1, dgBytes2, "raw drawing data (" + dgBytes1.length + " bytes) and aggregate.serialize() are different."); + assertArrayEquals(dgBytes1, dgBytes2, + "raw drawing data (" + dgBytes1.length + " bytes) and aggregate.serialize() are different."); } } diff --git a/test-data/spreadsheet/clusterfuzz-testcase-minimized-POIHSSFFuzzer-5436547081830400.xls b/test-data/spreadsheet/clusterfuzz-testcase-minimized-POIHSSFFuzzer-5436547081830400.xls new file mode 100644 index 0000000000..d8c01148ff Binary files /dev/null and b/test-data/spreadsheet/clusterfuzz-testcase-minimized-POIHSSFFuzzer-5436547081830400.xls differ diff --git a/test-data/spreadsheet/stress.xls b/test-data/spreadsheet/stress.xls index ce88487bdb..edc99459a8 100644 Binary files a/test-data/spreadsheet/stress.xls and b/test-data/spreadsheet/stress.xls differ -- cgit v1.2.3