From 1ff1e84e4afcd4abdf454c584a909423c2a14b03 Mon Sep 17 00:00:00 2001
From: Dominik Stadler <centic@apache.org>
Date: Tue, 3 Jan 2023 19:52:03 +0000
Subject: Avoid some NullPointerException and ClassCastExceptions found when
 fuzzing Apache POI

This mostly only makes thrown runtime-exceptions a bit more consistent and
improves information in exceptions.

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1906360 13f79535-47bb-0310-9956-ffa450edef68
---
 .../java/org/apache/poi/ddf/EscherBSERecord.java   |  7 ++-
 .../poi/hssf/extractor/OldExcelExtractor.java      | 17 +++++---
 .../apache/poi/hssf/model/InternalWorkbook.java    |  6 ++-
 .../hssf/record/aggregates/CFRecordsAggregate.java |  6 ++-
 .../apache/poi/hssf/usermodel/HSSFPatriarch.java   | 50 ++++++++++++++--------
 .../poi/hssf/usermodel/HSSFShapeFactory.java       | 18 ++++++--
 .../apache/poi/poifs/property/PropertyTable.java   |  8 +++-
 7 files changed, 81 insertions(+), 31 deletions(-)

(limited to 'poi')

diff --git a/poi/src/main/java/org/apache/poi/ddf/EscherBSERecord.java b/poi/src/main/java/org/apache/poi/ddf/EscherBSERecord.java
index 255fa212ec..4e37209115 100644
--- a/poi/src/main/java/org/apache/poi/ddf/EscherBSERecord.java
+++ b/poi/src/main/java/org/apache/poi/ddf/EscherBSERecord.java
@@ -111,8 +111,13 @@ public final class EscherBSERecord extends EscherRecord {
 
         int bytesRead = 0;
         if (bytesRemaining > 0) {
+            EscherRecord record = recordFactory.createRecord(data, pos + 36);
+            if (!(record instanceof EscherBlipRecord)) {
+                throw new IllegalArgumentException("Did not have a EscherBlipRecord: " + record);
+            }
+
             // Some older escher formats skip this last record
-            field_12_blipRecord = (EscherBlipRecord) recordFactory.createRecord( data, pos + 36 );
+            field_12_blipRecord = (EscherBlipRecord) record;
             bytesRead = field_12_blipRecord.fillFields( data, pos + 36, recordFactory );
         }
         pos += 36 + bytesRead;
diff --git a/poi/src/main/java/org/apache/poi/hssf/extractor/OldExcelExtractor.java b/poi/src/main/java/org/apache/poi/hssf/extractor/OldExcelExtractor.java
index ac1ca9541c..f6deb15a10 100644
--- a/poi/src/main/java/org/apache/poi/hssf/extractor/OldExcelExtractor.java
+++ b/poi/src/main/java/org/apache/poi/hssf/extractor/OldExcelExtractor.java
@@ -44,6 +44,7 @@ import org.apache.poi.hssf.record.RecordInputStream;
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
 import org.apache.poi.poifs.filesystem.DirectoryNode;
 import org.apache.poi.poifs.filesystem.DocumentNode;
+import org.apache.poi.poifs.filesystem.Entry;
 import org.apache.poi.poifs.filesystem.FileMagic;
 import org.apache.poi.poifs.filesystem.NotOLE2FileException;
 import org.apache.poi.poifs.filesystem.POIFSFileSystem;
@@ -149,14 +150,18 @@ public class OldExcelExtractor implements POITextExtractor {
     private void open(DirectoryNode directory) throws IOException {
         DocumentNode book;
         try {
-            book = (DocumentNode)directory.getEntry(OLD_WORKBOOK_DIR_ENTRY_NAME);
+            Entry entry = directory.getEntry(OLD_WORKBOOK_DIR_ENTRY_NAME);
+            if (!(entry instanceof DocumentNode)) {
+                throw new IllegalArgumentException("Did not have an Excel 5/95 Book stream: " + entry);
+            }
+            book = (DocumentNode) entry;
         } catch (FileNotFoundException | IllegalArgumentException e) {
             // some files have "Workbook" instead
-            book = (DocumentNode)directory.getEntry(WORKBOOK_DIR_ENTRY_NAMES.get(0));
-        }
-
-        if (book == null) {
-            throw new IOException("No Excel 5/95 Book stream found");
+            Entry entry = directory.getEntry(WORKBOOK_DIR_ENTRY_NAMES.get(0));
+            if (!(entry instanceof DocumentNode)) {
+                throw new IllegalArgumentException("Did not have an Excel 5/95 Book stream: " + entry);
+            }
+            book = (DocumentNode) entry;
         }
 
         ris = new RecordInputStream(directory.createDocumentInputStream(book));
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 7ca3b143ee..003ad45c6b 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
@@ -830,7 +830,11 @@ public final class InternalWorkbook {
 
         xfptr += index;
 
-        return ( ExtendedFormatRecord ) records.get(xfptr);
+        Record record = records.get(xfptr);
+        if (!(record instanceof ExtendedFormatRecord)) {
+            throw new IllegalStateException("Did not have a ExtendedFormatRecord: " + record);
+        }
+        return (ExtendedFormatRecord) record;
     }
 
     /**
diff --git a/poi/src/main/java/org/apache/poi/hssf/record/aggregates/CFRecordsAggregate.java b/poi/src/main/java/org/apache/poi/hssf/record/aggregates/CFRecordsAggregate.java
index bcf80e6189..bbfe353873 100644
--- a/poi/src/main/java/org/apache/poi/hssf/record/aggregates/CFRecordsAggregate.java
+++ b/poi/src/main/java/org/apache/poi/hssf/record/aggregates/CFRecordsAggregate.java
@@ -125,7 +125,11 @@ public final class CFRecordsAggregate extends RecordAggregate implements Generic
 
         CFRuleBase[] rules = new CFRuleBase[nRules];
         for (int i = 0; i < rules.length; i++) {
-            rules[i] = (CFRuleBase) rs.getNext();
+            Record record = rs.getNext();
+            if (!(record instanceof CFRuleBase)) {
+                throw new IllegalArgumentException("Did not have a CFRuleBase: " + record);
+            }
+            rules[i] = (CFRuleBase) record;
         }
 
         return new CFRecordsAggregate(header, rules);
diff --git a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFPatriarch.java b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFPatriarch.java
index 45ac29a9eb..876b6f245a 100644
--- a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFPatriarch.java
+++ b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFPatriarch.java
@@ -32,6 +32,7 @@ import org.apache.poi.ddf.EscherContainerRecord;
 import org.apache.poi.ddf.EscherDgRecord;
 import org.apache.poi.ddf.EscherOptRecord;
 import org.apache.poi.ddf.EscherProperty;
+import org.apache.poi.ddf.EscherRecord;
 import org.apache.poi.ddf.EscherSpRecord;
 import org.apache.poi.ddf.EscherSpgrRecord;
 import org.apache.poi.hssf.model.DrawingManager2;
@@ -80,9 +81,24 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing<HSSFShap
     HSSFPatriarch(HSSFSheet sheet, EscherAggregate boundAggregate) {
         _sheet = sheet;
         _boundAggregate = boundAggregate;
+
+        if (_boundAggregate == null || _boundAggregate.getEscherContainer() == null) {
+            throw new IllegalArgumentException("Could not read mainSpgrContainer from " + _boundAggregate);
+        }
+
         _mainSpgrContainer = _boundAggregate.getEscherContainer().getChildContainers().get(0);
-        EscherContainerRecord spContainer = (EscherContainerRecord) _boundAggregate.getEscherContainer()
-                .getChildContainers().get(0).getChild(0);
+
+        if (_mainSpgrContainer == null) {
+            throw new IllegalArgumentException("Could not read mainSpgrContainer from " + _boundAggregate);
+        }
+
+        EscherRecord child = _mainSpgrContainer.getChild(0);
+
+        if (!(child instanceof EscherContainerRecord)) {
+            throw new IllegalArgumentException("Did not have a EscherContainerRecord: " + child);
+        }
+
+        EscherContainerRecord spContainer = (EscherContainerRecord) child;
         _spgrRecord = spContainer.getChildById(EscherSpgrRecord.RECORD_ID);
         buildShapeTree();
     }
@@ -171,10 +187,10 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing<HSSFShap
     /**
      * Creates a simple shape.  This includes such shapes as lines, rectangles,
      * and ovals.
-     * 
-     * Note: Microsoft Excel seems to sometimes disallow 
-     * higher y1 than y2 or higher x1 than x2 in the anchor, you might need to 
-     * reverse them and draw shapes vertically or horizontally flipped! 
+     *
+     * Note: Microsoft Excel seems to sometimes disallow
+     * higher y1 than y2 or higher x1 than x2 in the anchor, you might need to
+     * reverse them and draw shapes vertically or horizontally flipped!
      *
      * @param anchor the client anchor describes how this group is attached
      *               to the sheet.
@@ -234,14 +250,14 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing<HSSFShap
         ftCmo.setReserved2(0);
         ftCmo.setReserved3(0);
         obj.addSubRecord(ftCmo);
-        
-        // FtCf (pictFormat) 
+
+        // FtCf (pictFormat)
         FtCfSubRecord ftCf = new FtCfSubRecord();
         HSSFPictureData pictData = getSheet().getWorkbook().getAllPictures().get(pictureIndex-1);
         switch (pictData.getFormat()) {
             case Workbook.PICTURE_TYPE_WMF:
             case Workbook.PICTURE_TYPE_EMF:
-                // this needs patch #49658 to be applied to actually work 
+                // this needs patch #49658 to be applied to actually work
                 ftCf.setFlags(FtCfSubRecord.METAFILE_BIT);
                 break;
             case Workbook.PICTURE_TYPE_DIB:
@@ -258,12 +274,12 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing<HSSFShap
         FtPioGrbitSubRecord ftPioGrbit = new FtPioGrbitSubRecord();
         ftPioGrbit.setFlagByBit(FtPioGrbitSubRecord.AUTO_PICT_BIT, true);
         obj.addSubRecord(ftPioGrbit);
-        
+
         EmbeddedObjectRefSubRecord ftPictFmla = new EmbeddedObjectRefSubRecord();
         ftPictFmla.setUnknownFormulaData(new byte[]{2, 0, 0, 0, 0});
         ftPictFmla.setOleClassname("Paket");
         ftPictFmla.setStorageId(storageId);
-        
+
         obj.addSubRecord(ftPictFmla);
         obj.addSubRecord(new EndSubRecord());
 
@@ -278,22 +294,22 @@ public final class HSSFPatriarch implements HSSFShapeContainer, Drawing<HSSFShap
         } catch (FileNotFoundException e) {
             throw new IllegalStateException("trying to add ole shape without actually adding data first - use HSSFWorkbook.addOlePackage first", e);
         }
-        
+
         // create picture shape, which need to be minimal modified for oleshapes
         HSSFPicture shape = new HSSFPicture(null, (HSSFClientAnchor)anchor);
         shape.setPictureIndex(pictureIndex);
         EscherContainerRecord spContainer = shape.getEscherContainer();
         EscherSpRecord spRecord = spContainer.getChildById(EscherSpRecord.RECORD_ID);
         spRecord.setFlags(spRecord.getFlags() |  EscherSpRecord.FLAG_OLESHAPE);
-        
-        HSSFObjectData oleShape = new HSSFObjectData(spContainer, obj, oleRoot); 
+
+        HSSFObjectData oleShape = new HSSFObjectData(spContainer, obj, oleRoot);
         addShape(oleShape);
         onCreate(oleShape);
-        
-        
+
+
         return oleShape;
     }
-    
+
     /**
      * Creates a polygon
      *
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 88c0a7d4d3..fff9250a68 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
@@ -69,12 +69,22 @@ public class HSSFShapeFactory {
 
             for (EscherRecord record : container) {
                 switch (EscherRecordTypes.forTypeID(record.getRecordId())) {
-                    case CLIENT_DATA:
-                        objRecord = (ObjRecord) shapeToObj.get(record);
+                    case CLIENT_DATA: {
+                        Record subRecord = shapeToObj.get(record);
+                        if (!(subRecord instanceof ObjRecord)) {
+                            throw new RecordFormatException("Did not have a ObjRecord: " + subRecord);
+                        }
+                        objRecord = (ObjRecord) subRecord;
                         break;
-                    case CLIENT_TEXTBOX:
-                        txtRecord = (TextObjectRecord) shapeToObj.get(record);
+                    }
+                    case CLIENT_TEXTBOX: {
+                        Record subRecord = shapeToObj.get(record);
+                        if (!(subRecord instanceof TextObjectRecord)) {
+                            throw new RecordFormatException("Did not have a TextObjRecord: " + subRecord);
+                        }
+                        txtRecord = (TextObjectRecord) subRecord;
                         break;
+                    }
                     default:
                         break;
                 }
diff --git a/poi/src/main/java/org/apache/poi/poifs/property/PropertyTable.java b/poi/src/main/java/org/apache/poi/poifs/property/PropertyTable.java
index 07e5187a4b..5e9565da87 100644
--- a/poi/src/main/java/org/apache/poi/poifs/property/PropertyTable.java
+++ b/poi/src/main/java/org/apache/poi/poifs/property/PropertyTable.java
@@ -140,7 +140,13 @@ public final class PropertyTable implements BATManaged {
      */
     public RootProperty getRoot() {
         // it's always the first element in the List
-        return ( RootProperty ) _properties.get(0);
+        Property property = _properties.get(0);
+        if (property instanceof RootProperty) {
+            return (RootProperty) property;
+        } else {
+            throw new IllegalStateException("Invalid format, cannot convert property " +
+                    property + " to RootProperty");
+        }
     }
 
     /**
-- 
cgit v1.2.3