Fix issues found when fuzzing Apache POI via Jazzer

Add some null-checks and report more meaningful exceptions
This provides a bit more information than simple NullPointExceptions

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1895600 13f79535-47bb-0310-9956-ffa450edef68

poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/internal/unmarshallers/PackagePropertiesUnmarshaller.java

@@ -240,45 +240,52 @@ public final class PackagePropertiesUnmarshaller implements PartUnmarshaller {
         for (int i = 0; i < namedNodeCount; i++) {
             Attr attr = (Attr)namedNodeMap.item(0);
 
-            if (attr.getNamespaceURI().equals(XMLConstants.XMLNS_ATTRIBUTE_NS_URI)) {
+            if (attr != null && XMLConstants.XMLNS_ATTRIBUTE_NS_URI.equals(attr.getNamespaceURI())) {
                 // Rule M4.2
-                if (attr.getValue().equals(PackageNamespaces.MARKUP_COMPATIBILITY))
+                if (PackageNamespaces.MARKUP_COMPATIBILITY.equals(attr.getValue())) {
                     throw new InvalidFormatException(
                             "OPC Compliance error [M4.2]: A format consumer shall consider the use of the Markup Compatibility namespace to be an error.");
-
+                }
             }
         }
 
         // Rule M4.3
         String elName = el.getLocalName();
-        if (el.getNamespaceURI().equals(PackageProperties.NAMESPACE_DCTERMS))
-            if (!(elName.equals(KEYWORD_CREATED) || elName.equals(KEYWORD_MODIFIED)))
+        if (PackageProperties.NAMESPACE_DCTERMS.equals(el.getNamespaceURI())) {
+            if (!(KEYWORD_CREATED.equals(elName) || KEYWORD_MODIFIED.equals(elName))) {
                 throw new InvalidFormatException(
                         "OPC Compliance error [M4.3]: Producers shall not create a document element that contains refinements to the Dublin Core elements, except for the two specified in the schema: <dcterms:created> and <dcterms:modified> Consumers shall consider a document element that violates this constraint to be an error.");
+            }
+        }
 
         // Rule M4.4
-        if (el.getAttributeNodeNS(XMLConstants.XML_NS_URI, "lang") != null)
+        if (el.getAttributeNodeNS(XMLConstants.XML_NS_URI, "lang") != null) {
             throw new InvalidFormatException(
                     "OPC Compliance error [M4.4]: Producers shall not create a document element that contains the xml:lang attribute. Consumers shall consider a document element that violates this constraint to be an error.");
+        }
 
         // Rule M4.5
-        if (el.getNamespaceURI().equals(PackageProperties.NAMESPACE_DCTERMS)) {
+        if (PackageProperties.NAMESPACE_DCTERMS.equals(el.getNamespaceURI())) {
             // DCTerms namespace only use with 'created' and 'modified' elements
-            if (!(elName.equals(KEYWORD_CREATED) || elName.equals(KEYWORD_MODIFIED)))
+            if (!(elName.equals(KEYWORD_CREATED) || elName.equals(KEYWORD_MODIFIED))) {
                 throw new InvalidFormatException("Namespace error : " + elName
                         + " shouldn't have the following naemspace -> "
                         + PackageProperties.NAMESPACE_DCTERMS);
+            }
 
             // Check for the 'xsi:type' attribute
             Attr typeAtt = el.getAttributeNodeNS(XMLConstants.W3C_XML_SCHEMA_INSTANCE_NS_URI, "type");
-            if (typeAtt == null)
+            if (typeAtt == null) {
                 throw new InvalidFormatException("The element '" + elName
                         + "' must have the 'xsi:type' attribute present !");
+            }
 
             // Check for the attribute value => 'dcterms:W3CDTF'
-            if (!typeAtt.getValue().equals(el.getPrefix() + ":W3CDTF"))
+            if (!typeAtt.getValue().equals(el.getPrefix() + ":W3CDTF")) {
                 throw new InvalidFormatException("The element '" + elName
-                        + "' must have the 'xsi:type' attribute with the value '" + el.getPrefix() + ":W3CDTF', but had '" + typeAtt.getValue() + "' !");
+                        + "' must have the 'xsi:type' attribute with the value '" + el.getPrefix() + ":W3CDTF', but had '"
+                        + typeAtt.getValue() + "' !");
+            }
         }
 
         // Check its children

poi-ooxml/src/main/java/org/apache/poi/xdgf/usermodel/XDGFPageContents.java

@@ -55,6 +55,9 @@ public class XDGFPageContents extends XDGFBaseContents {
                 //throw new POIXMLException("Unexpected page relation: " + part);
 
                 XDGFMaster master = ((XDGFMasterContents)part).getMaster();
+                if (master == null) {
+                    throw new POIXMLException("Master entry is missing in XDGFPageContents");
+                }
                 _masters.put(master.getID(), master);
             }
 

poi-ooxml/src/main/java/org/apache/poi/xdgf/usermodel/XmlVisioDocument.java

@@ -38,18 +38,18 @@ import com.microsoft.schemas.office.visio.x2012.main.VisioDocumentType;
 /**
  * This is your high-level starting point for working with Visio XML
  * documents (.vsdx).
- * 
+ *
  * Currently, only read support has been implemented, and the API is
  * not mature and is subject to change.
- * 
+ *
  * For more information about the visio XML format (with an XSD 1.0
  * schema), you can find documentation at
  * https://msdn.microsoft.com/en-us/library/hh645006(v=office.12).aspx
- * 
+ *
  * That document lacks in some areas, but you can find additional
  * documentation and an updated XSD 1.1 schema at
  * https://msdn.microsoft.com/en-us/library/office/jj684209(v=office.15).aspx
- * 
+ *
  * Each provides different details, but the SharePoint reference
  * has better documentation and is more useful.
  */
@@ -101,7 +101,9 @@ public class XmlVisioDocument extends POIXMLDocument {
             _masters.onDocumentRead();
         }
 
-        _pages.onDocumentRead();
+        if (_pages != null) {
+            _pages.onDocumentRead();
+        }
     }
 
     /**
@@ -115,7 +117,7 @@ public class XmlVisioDocument extends POIXMLDocument {
     //
     // Useful public API goes here
     //
-    
+
     /**
      * @return pages ordered by page number
      */

poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFSheet.java

@@ -87,7 +87,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet, OoxmlSheetEx
     private static final double DEFAULT_MARGIN_BOTTOM = 0.75;
     private static final double DEFAULT_MARGIN_LEFT = 0.7;
     private static final double DEFAULT_MARGIN_RIGHT = 0.7;
-    
+
     //TODO make the two variable below private!
     protected CTSheet sheet;
     protected CTWorksheet worksheet;
@@ -192,6 +192,10 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet, OoxmlSheetEx
     }
 
     private void initRows(CTWorksheet worksheetParam) {
+        if (worksheetParam.getSheetData() == null || worksheetParam.getSheetData().getRowArray() == null) {
+            throw new IllegalArgumentException("Had empty sheet data when initializing the sheet");
+        }
+
         _rows.clear();
         tables = new TreeMap<>();
         sharedFormulas = new HashMap<>();

poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java

@@ -403,6 +403,12 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook, Date1904Su
             // Load individual sheets. The order of sheets is defined by the order
             //  of CTSheet elements in the workbook
             sheets = new ArrayList<>(shIdMap.size());
+
+            if (this.workbook == null || this.workbook.getSheets() == null ||
+                    this.workbook.getSheets().getSheetArray() == null) {
+                throw new POIXMLException("Cannot read a workbook without sheets");
+            }
+
             for (CTSheet ctSheet : this.workbook.getSheets().getSheetArray()) {
                 parseSheet(shIdMap, ctSheet);
             }
@@ -671,22 +677,22 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook, Date1904Su
             clonedDg.getCTDrawing().set(dg.getCTDrawing().copy());
 
             // Clone drawing relations
-			XSSFDrawing drawingPatriarch = srcSheet.getDrawingPatriarch();
-			if (drawingPatriarch != null) {
-				List<RelationPart> srcRels = drawingPatriarch.getRelationParts();
-				for (RelationPart rp : srcRels) {
-					POIXMLDocumentPart r = rp.getDocumentPart();
-					if (r instanceof XSSFChart) {
-						// Replace chart relation part with new relationship, cloning the chart's content
-						RelationPart chartPart = clonedDg.createChartRelationPart();
-						XSSFChart chart = chartPart.getDocumentPart();
-						chart.importContent((XSSFChart) r);
-						chart.replaceReferences(clonedSheet);
-					} else {
-						addRelation(rp, clonedDg);
-					}
-				}
-			}
+            XSSFDrawing drawingPatriarch = srcSheet.getDrawingPatriarch();
+            if (drawingPatriarch != null) {
+                List<RelationPart> srcRels = drawingPatriarch.getRelationParts();
+                for (RelationPart rp : srcRels) {
+                    POIXMLDocumentPart r = rp.getDocumentPart();
+                    if (r instanceof XSSFChart) {
+                        // Replace chart relation part with new relationship, cloning the chart's content
+                        RelationPart chartPart = clonedDg.createChartRelationPart();
+                        XSSFChart chart = chartPart.getDocumentPart();
+                        chart.importContent((XSSFChart) r);
+                        chart.replaceReferences(clonedSheet);
+                    } else {
+                        addRelation(rp, clonedDg);
+                    }
+                }
+            }
         }
         return clonedSheet;
     }
@@ -875,11 +881,11 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook, Date1904Su
         if(sheetname.length() > MAX_SENSITIVE_SHEET_NAME_LEN) {
             String trimmedSheetname = sheetname.substring(0, MAX_SENSITIVE_SHEET_NAME_LEN);
 
-			// we still need to warn about the trimming as the original sheet name won't be available
-			// e.g. when referenced by formulas
-			LOG.atWarn().log("Sheet '{}' will be added with a trimmed name '{}' for MS Excel compliance.",
-					sheetname, trimmedSheetname);
-			sheetname = trimmedSheetname;
+            // we still need to warn about the trimming as the original sheet name won't be available
+            // e.g. when referenced by formulas
+            LOG.atWarn().log("Sheet '{}' will be added with a trimmed name '{}' for MS Excel compliance.",
+                    sheetname, trimmedSheetname);
+            sheetname = trimmedSheetname;
         }
         WorkbookUtil.validateSheetName(sheetname);
 

poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSlideMaster.java

@@ -143,14 +143,23 @@ public final class HSLFSlideMaster extends HSLFMasterSheet {
         assert (_txmaster == null);
         _txmaster = new TxMasterStyleAtom[9];
 
+        if (getSlideShow() == null || getSlideShow().getDocumentRecord() == null ||
+                getSlideShow().getDocumentRecord().getEnvironment() == null) {
+            throw new IllegalStateException("Did not find a TxMasterStyleAtom in the current slide show");
+        }
+
         TxMasterStyleAtom txdoc = getSlideShow().getDocumentRecord().getEnvironment().getTxMasterStyleAtom();
+        if (txdoc == null) {
+            throw new IllegalStateException("Did not find a TxMasterStyleAtom in the current slide show");
+        }
+
         _txmaster[txdoc.getTextType()] = txdoc;
 
         TxMasterStyleAtom[] txrec = ((MainMaster)getSheetContainer()).getTxMasterStyleAtoms();
-        for (int i = 0; i < txrec.length; i++) {
-            int txType = txrec[i].getTextType();
+        for (TxMasterStyleAtom txMasterStyleAtom : txrec) {
+            int txType = txMasterStyleAtom.getTextType();
             if (txType < _txmaster.length && _txmaster[txType] == null) {
-                _txmaster[txType] = txrec[i];
+                _txmaster[txType] = txMasterStyleAtom;
             }
         }
 

poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSlideShow.java

@@ -276,7 +276,9 @@ public final class HSLFSlideShow extends POIDocument implements SlideShow<HSLFSh
                 // Find the Document, and interesting things in it
                 if (record.getRecordType() == RecordTypes.Document.typeID) {
                     _documentRecord = (Document) record;
-                    _fonts = _documentRecord.getEnvironment().getFontCollection();
+                    if (_documentRecord.getEnvironment() != null) {
+                        _fonts = _documentRecord.getEnvironment().getFontCollection();
+                    }
                 }
             } /*else {
                 // No record at this number
@@ -414,6 +416,10 @@ public final class HSLFSlideShow extends POIDocument implements SlideShow<HSLFSh
             int slideId = spa.getSlideIdentifier();
             slideIdToNotes.put(slideId, idx);
 
+            if (notesRecord.getNotesAtom() == null) {
+                throw new IllegalStateException("Could not read NotesAtom from the NotesRecord for " + idx);
+            }
+
             HSLFNotes hn = new HSLFNotes(notesRecord);
             hn.setSlideShow(this);
             _notes.add(hn);
@@ -436,11 +442,16 @@ public final class HSLFSlideShow extends POIDocument implements SlideShow<HSLFSh
 
             // Ensure it really is a slide record
             if (!(r instanceof Slide)) {
-                LOG.atError().log("A Slide SlideAtomSet at {} said its record was at refID {}, but that was actually a {}", box(idx),box(spa.getRefID()),r);
+                LOG.atError().log("A Slide SlideAtomSet at {} said its record was at refID {}, but that was actually a {}",
+                        box(idx), box(spa.getRefID()), r);
                 continue;
             }
 
             Slide slide = (Slide)r;
+            if (slide.getSlideAtom() == null) {
+                LOG.atError().log("SlideAtomSet at {} at refID {} is null", box(idx), box(spa.getRefID()));
+                continue;
+            }
 
             // Do we have a notes for this?
             HSLFNotes notes = null;

poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSlideShowImpl.java

@@ -310,10 +310,17 @@ public final class HSLFSlideShowImpl extends POIDocument implements Closeable {
     private void initRecordOffsets(byte[] docstream, int usrOffset, NavigableMap<Integer, Record> recordMap, Map<Integer, Integer> offset2id) {
         while (usrOffset != 0) {
             UserEditAtom usr = (UserEditAtom) Record.buildRecordAtOffset(docstream, usrOffset);
+            if (usr == null) {
+                throw new CorruptPowerPointFileException("Powerpoint document contains no user edit atom");
+            }
+
             recordMap.put(usrOffset, usr);
 
             int psrOffset = usr.getPersistPointersOffset();
             PersistPtrHolder ptr = (PersistPtrHolder) Record.buildRecordAtOffset(docstream, psrOffset);
+            if (ptr == null) {
+                throw new CorruptPowerPointFileException("Powerpoint document is missing a PersistPtrHolder at " + psrOffset);
+            }
             recordMap.put(psrOffset, ptr);
 
             for (Map.Entry<Integer, Integer> entry : ptr.getSlideLocationsLookup().entrySet()) {

poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFTextParagraph.java

@@ -1322,14 +1322,18 @@ public final class HSLFTextParagraph implements TextParagraph<HSLFShape,HSLFText
      * For a given PPDrawing, grab all the TextRuns
      */
    public static List<List<HSLFTextParagraph>> findTextParagraphs(PPDrawing ppdrawing, HSLFSheet sheet) {
-        List<List<HSLFTextParagraph>> runsV = new ArrayList<>();
-        for (EscherTextboxWrapper wrapper : ppdrawing.getTextboxWrappers()) {
-            List<HSLFTextParagraph> p = findTextParagraphs(wrapper, sheet);
-            if (p != null) {
-                runsV.add(p);
-            }
-        }
-        return runsV;
+       if (ppdrawing == null) {
+           throw new IllegalArgumentException("Did not receive a valid drawing for sheet " + sheet._getSheetNumber());
+       }
+
+       List<List<HSLFTextParagraph>> runsV = new ArrayList<>();
+       for (EscherTextboxWrapper wrapper : ppdrawing.getTextboxWrappers()) {
+           List<HSLFTextParagraph> p = findTextParagraphs(wrapper, sheet);
+           if (p != null) {
+               runsV.add(p);
+           }
+       }
+       return runsV;
     }
 
     /**

poi/src/main/java/org/apache/poi/poifs/crypt/agile/AgileDecryptor.java

@@ -196,6 +196,10 @@ public class AgileDecryptor extends Decryptor {
         Cipher cipher = getCipher(skey, cipherAlgo, chainMode, iv, cipherMode);
         byte[] hashFinal;
 
+        if (inputKey == null) {
+            throw new EncryptedDocumentException("Cannot has input without inputKey");
+        }
+
         try {
             inputKey = getBlock0(inputKey, getNextBlockSize(inputKey.length, blockSize));
             hashFinal = cipher.doFinal(inputKey);

poi/src/main/java/org/apache/poi/poifs/crypt/agile/AgileEncryptionVerifier.java

@@ -45,7 +45,7 @@ public class AgileEncryptionVerifier extends EncryptionVerifier {
         }
 
         if (keyData == null) {
-            throw new NullPointerException("encryptedKey not set");
+            throw new IllegalArgumentException("encryptedKey not set");
         }
 
         setCipherAlgorithm(keyData.getCipherAlgorithm());
@@ -64,14 +64,17 @@ public class AgileEncryptionVerifier extends EncryptionVerifier {
                     keyData.getHashAlgorithm() + " @ " + hashSize + " bytes");
         }
 
-        setSpinCount(keyData.getSpinCount());
+        Integer spinCount = keyData.getSpinCount();
+        if (spinCount != null) {
+            setSpinCount(spinCount);
+        }
         setEncryptedVerifier(keyData.getEncryptedVerifierHashInput());
         setSalt(keyData.getSaltValue());
         setEncryptedKey(keyData.getEncryptedKeyValue());
         setEncryptedVerifierHash(keyData.getEncryptedVerifierHashValue());
 
-        int saltSize = keyData.getSaltSize();
-        if (saltSize != getSalt().length) {
+        Integer saltSize = keyData.getSaltSize();
+        if (saltSize == null || saltSize != getSalt().length) {
             throw new EncryptedDocumentException("Invalid salt size");
         }
 

poi/src/main/java/org/apache/poi/poifs/filesystem/POIFSMiniStore.java

@@ -29,6 +29,7 @@ import org.apache.poi.poifs.property.RootProperty;
 import org.apache.poi.poifs.storage.BATBlock;
 import org.apache.poi.poifs.storage.BATBlock.BATBlockAndIndex;
 import org.apache.poi.poifs.storage.HeaderBlock;
+import org.apache.poi.util.RecordFormatException;
 
 /**
  * This class handles the MiniStream (small block store)
@@ -43,6 +44,9 @@ public class POIFSMiniStore extends BlockStore {
 
     POIFSMiniStore(POIFSFileSystem filesystem, RootProperty root,
                    List<BATBlock> sbats, HeaderBlock header) {
+        if (root == null) {
+            throw new RecordFormatException("Invalid argument to POIFSMiniStore: root is null");
+        }
         this._filesystem = filesystem;
         this._sbat_blocks = sbats;
         this._header = header;

poi/src/main/java/org/apache/poi/poifs/property/PropertyTable.java

@@ -105,7 +105,9 @@ public final class PropertyTable implements BATManaged {
             PropertyFactory.convertToProperties(data, _properties);
         }
 
-        populatePropertyTree( (DirectoryProperty)_properties.get(0));
+        if (_properties.get(0) != null) {
+            populatePropertyTree((DirectoryProperty) _properties.get(0));
+        }
     }