Fix issues found when fuzzing Apache POI via Jazzer

Replace assertions with actual checks when input-data
can trigger them. We would not handle such
input-data properly otherwise.
Sometimes logging seems a better option if the issue
is not blocking us from parsing the document anyway

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

diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/PPDrawing.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/PPDrawing.java
index 1267da2eebe8fb328201e47b4affd696f3553802..e5d86ef930f04f5cce8d6515a70f389fb49f125e 100644 (file)
--- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/PPDrawing.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/PPDrawing.java
@@ -129,7 +129,9 @@ public final class PPDrawing extends RecordAtom implements Iterable<EscherRecord
         // Build up a tree of Escher records contained within
         final DefaultEscherRecordFactory erf = new HSLFEscherRecordFactory();
         dgContainer.fillFields(source, start + 8, erf);
-        assert dgContainer.getRecordId() == EscherRecordTypes.DG_CONTAINER.typeID;
+               if (dgContainer.getRecordId() != EscherRecordTypes.DG_CONTAINER.typeID) {
+                       throw new IllegalArgumentException("Unexpected record type: " + dgContainer.getRecordId());
+               }
         dg = dgContainer.getChildById(EscherRecordTypes.DG.typeID);
 
         textboxWrappers = Stream.of(dgContainer).

diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/SSSlideInfoAtom.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/SSSlideInfoAtom.java
index 2aa1cafc6322972b8f22ae3bad33737cc86a66e6..399f21b104d5c98f00ea0329d9bb93fd1d6e26dd 100644 (file)
--- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/SSSlideInfoAtom.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/SSSlideInfoAtom.java
@@ -198,13 +198,23 @@ public class SSSlideInfoAtom extends RecordAtom {
         _header = Arrays.copyOfRange(source, ofs, ofs+8);
         ofs += _header.length;
 
-        assert(LittleEndian.getShort(_header, 0) == 0);
-        assert(LittleEndian.getShort(_header, 2) == RecordTypes.SSSlideInfoAtom.typeID);
-        assert(LittleEndian.getShort(_header, 4) == 0x10);
-        assert(LittleEndian.getShort(_header, 6) == 0);
+        if (LittleEndian.getShort(_header, 0) != 0) {
+                       LOG.atDebug().log("Invalid data for SSSlideInfoAtom at offset 0: " + LittleEndian.getShort(_header, 0));
+               }
+               if (LittleEndian.getShort(_header, 2) != RecordTypes.SSSlideInfoAtom.typeID) {
+                       LOG.atDebug().log("Invalid data for SSSlideInfoAtom at offset 2: "+ LittleEndian.getShort(_header, 2));
+               }
+               if (LittleEndian.getShort(_header, 4) != 0x10) {
+                       LOG.atDebug().log("Invalid data for SSSlideInfoAtom at offset 4: "+ LittleEndian.getShort(_header, 4));
+               }
+               if (LittleEndian.getShort(_header, 6) == 0) {
+                       LOG.atDebug().log("Invalid data for SSSlideInfoAtom at offset 6: "+ LittleEndian.getShort(_header, 6));
+               }
 
         _slideTime = LittleEndian.getInt(source, ofs);
-        assert(0 <= _slideTime && _slideTime <= 86399000);
+               if (_slideTime < 0 || _slideTime > 86399000) {
+                       LOG.atDebug().log("Invalid data for SSSlideInfoAtom - invalid slideTime: "+ _slideTime);
+               }
         ofs += LittleEndianConsts.INT_SIZE;
         _soundIdRef = LittleEndian.getInt(source, ofs);
         ofs += LittleEndianConsts.INT_SIZE;

diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/UserEditAtom.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/UserEditAtom.java
index 898e22dae259e45c2e04c623860c3082845bc523..3dbee3c4b611428c1dfab7c96bd1db8f69bea7f1 100644 (file)
--- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/UserEditAtom.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/UserEditAtom.java
@@ -136,7 +136,10 @@ public final class UserEditAtom extends PositionDependentRecordAtom
             offset += LittleEndianConsts.INT_SIZE;
         }
 
-        assert(offset-start == len);
+        if(offset-start != len) {
+                       throw new HSLFException("Having invalid data in UserEditAtom: "
+                                       + "len: " + len + ", offset: " + offset + ", start: " + start);
+               }
     }
 
     /**

diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSlideShowEncrypted.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSlideShowEncrypted.java
index 56d1e5440cf030c0a7b0c3a6e81dc21f9533913c..273f9e87ed660441c50bd2ff650ebb589891335b 100644 (file)
--- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSlideShowEncrypted.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSlideShowEncrypted.java
@@ -73,7 +73,7 @@ public class HSLFSlideShowEncrypted implements Closeable {
             1,     // unused2
             1,     // unused3
     };
-    
+
     protected HSLFSlideShowEncrypted(DocumentEncryptionAtom dea) {
         this.dea = dea;
     }
@@ -116,8 +116,8 @@ public class HSLFSlideShowEncrypted implements Closeable {
             r = Record.buildRecordAtOffset(docstream, encOffset);
             recordMap.put(encOffset, r);
         }
-        assert(r instanceof DocumentEncryptionAtom);
-        this.dea = (DocumentEncryptionAtom)r;
+
+               this.dea = (DocumentEncryptionAtom)r;
 
         String pass = Biff8EncryptionKey.getCurrentUserPassword();
         EncryptionInfo ei = getEncryptionInfo();
@@ -205,7 +205,7 @@ public class HSLFSlideShowEncrypted implements Closeable {
         ccis.close();
         lei.close();
     }
-    
+
     protected void decryptPicture(byte[] pictstream, int offset) {
         if (dea == null) {
             return;
@@ -229,14 +229,14 @@ public class HSLFSlideShowEncrypted implements Closeable {
                     decryptPicBytes(pictstream, offset, part);
                 }
                 offset += 36;
-                
+
                 int cbName = LittleEndian.getUShort(pictstream, offset-3);
                 if (cbName > 0) {
                     // read nameData
                     decryptPicBytes(pictstream, offset, cbName);
                     offset += cbName;
                 }
-                
+
                 if (offset == endOffset) {
                     return; // no embedded blip
                 }
@@ -267,7 +267,7 @@ public class HSLFSlideShowEncrypted implements Closeable {
                 // tag
                 nextBytes = 1;
             }
-            
+
             decryptPicBytes(pictstream, offset, nextBytes);
             offset += nextBytes;
 
@@ -304,19 +304,19 @@ public class HSLFSlideShowEncrypted implements Closeable {
 
                 // File BLIP Store Entry (FBSE)
                 int cbName = LittleEndian.getUShort(pictstream, offset+33);
-                
+
                 for (int part : BLIB_STORE_ENTRY_PARTS) {
                     ccos.write(pictstream, offset, part);
                     ccos.flush();
                     offset += part;
                 }
-                
+
                 if (cbName > 0) {
                     ccos.write(pictstream, offset, cbName);
                     ccos.flush();
                     offset += cbName;
                 }
-                
+
                 if (offset == endOffset) {
                     return; // no embedded blip
                 }

diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/OfficeArtContent.java b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/OfficeArtContent.java
index d8c13132b74163aed145e5a87be7ed88f1e8ac70..52b89f3212a83e0f1ec6bf71cc932961bdd41a7e 100644 (file)
--- a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/OfficeArtContent.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/OfficeArtContent.java
@@ -21,6 +21,7 @@ import java.util.ArrayList;
 import java.util.List;
 
 import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.apache.poi.ddf.DefaultEscherRecordFactory;
 import org.apache.poi.ddf.EscherContainerRecord;
 import org.apache.poi.ddf.EscherRecord;
@@ -37,6 +38,7 @@ import static org.apache.logging.log4j.util.Unbox.box;
  */
 @Internal
 public final class OfficeArtContent {
+       protected static final Logger LOG = LogManager.getLogger(OfficeArtContent.class);
 
     /**
      * {@link EscherRecordTypes#DGG_CONTAINER} containing drawing group information for the document.
@@ -76,7 +78,9 @@ public final class OfficeArtContent {
         EscherRecordFactory recordFactory = new DefaultEscherRecordFactory();
         int pos = offset;
         pos += drawingGroupData.fillFields(data, pos, recordFactory);
-        assert drawingGroupData.getRecordId() == EscherRecordTypes.DGG_CONTAINER.typeID;
+               if (drawingGroupData.getRecordId() == EscherRecordTypes.DGG_CONTAINER.typeID) {
+                       LOG.atDebug().log("Invalid record-id for filling Escher records: " + drawingGroupData.getRecordId());
+               }
 
         /*
          * After the drawingGroupData there is an array (2 slots max) that has data about drawings. According to the
@@ -92,12 +96,18 @@ public final class OfficeArtContent {
 
             // Named this way to match section 2.9.172 of [MS-DOC] - v20191119.
             byte dgglbl = data[pos];
-            assert dgglbl == 0x00 || dgglbl == 0x01;
+
+            if (dgglbl != 0x00 && dgglbl != 0x01) {
+                               throw new IllegalArgumentException("Invalid dgglbl when filling Escher records: " + dgglbl);
+                       }
             pos++;
 
             EscherContainerRecord dgContainer = new EscherContainerRecord();
             pos+= dgContainer.fillFields(data, pos, recordFactory);
-            assert dgContainer.getRecordId() == EscherRecordTypes.DG_CONTAINER.typeID;
+                       if (dgContainer.getRecordId() != EscherRecordTypes.DG_CONTAINER.typeID) {
+                               throw new IllegalArgumentException("Did have an invalid record-type: " + dgContainer.getRecordId() +
+                                               " when filling Escher records");
+                       }
 
             switch (dgglbl) {
                 case 0x00:
@@ -112,7 +122,10 @@ public final class OfficeArtContent {
             }
         }
 
-        assert pos == offset + size;
+               if (pos != offset + size) {
+                       throw new IllegalStateException("Did not read all data when filling Escher records: "
+                                       + "pos: " + pos + ", offset: " + offset + ", size: " + size);
+               }
     }
 
     private List<? extends EscherContainerRecord> getDgContainers() {

diff --git a/poi/src/main/java/org/apache/poi/hssf/record/RecordInputStream.java b/poi/src/main/java/org/apache/poi/hssf/record/RecordInputStream.java
index e62431c672b236f7a34d20c4131035baaa035304..1ff551c33297061fd5be2bd6578bfc028ac541c2 100644 (file)
--- a/poi/src/main/java/org/apache/poi/hssf/record/RecordInputStream.java
+++ b/poi/src/main/java/org/apache/poi/hssf/record/RecordInputStream.java
@@ -420,7 +420,9 @@ public final class RecordInputStream implements LittleEndianInput {
             nextRecord();
             // note - the compressed flag may change on the fly
             byte compressFlag = readByte();
-            assert(compressFlag == 0 || compressFlag == 1);
+                       if (compressFlag != 0 && compressFlag != 1) {
+                               throw new RecordFormatException("Invalid compressFlag: " + compressFlag);
+                       }
             isCompressedEncoding = (compressFlag == 0);
         }
     }