From a7810011c70585dbd1fc4eab30dd49cf96b1707e Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Sun, 20 Mar 2022 06:52:38 +0000 Subject: [PATCH] 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 --- .../org/apache/poi/hslf/record/PPDrawing.java | 4 +++- .../poi/hslf/record/SSSlideInfoAtom.java | 20 +++++++++++++----- .../apache/poi/hslf/record/UserEditAtom.java | 5 ++++- .../usermodel/HSLFSlideShowEncrypted.java | 20 +++++++++--------- .../poi/hwpf/model/OfficeArtContent.java | 21 +++++++++++++++---- .../poi/hssf/record/RecordInputStream.java | 4 +++- 6 files changed, 52 insertions(+), 22 deletions(-) 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 1267da2eeb..e5d86ef930 100644 --- 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 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 898e22dae2..3dbee3c4b6 100644 --- 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 56d1e5440c..273f9e87ed 100644 --- 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 d8c13132b7..52b89f3212 100644 --- 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 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 e62431c672..1ff551c332 100644 --- 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); } } -- 2.39.5