From f0e7cc0881856ba25a965504e68a70f7dd9046b3 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Sun, 5 Dec 2021 17:34:19 +0000 Subject: Fix issues found when fuzzing Apache POI via Jazzer Check for negative array allocation size or access and report a more meaningful exception git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1895599 13f79535-47bb-0310-9956-ffa450edef68 --- .../java/org/apache/poi/hdgf/streams/CompressedStreamStore.java | 6 +++++- .../src/main/java/org/apache/poi/hpbf/model/qcbits/QCPLCBit.java | 7 +++++-- poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/Ffn.java | 5 +++++ .../src/main/java/org/apache/poi/hwpf/model/LFOData.java | 7 +++++-- .../src/main/java/org/apache/poi/hwpf/model/StyleSheet.java | 4 ++++ poi/src/main/java/org/apache/poi/hssf/record/RecordFactory.java | 8 +++++++- .../main/java/org/apache/poi/hssf/record/RecordInputStream.java | 5 ++++- .../java/org/apache/poi/hssf/record/chart/ChartFRTInfoRecord.java | 3 +++ poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFRow.java | 4 ++++ poi/src/main/java/org/apache/poi/poifs/filesystem/BlockStore.java | 4 ++++ .../org/apache/poi/ss/formula/constant/ConstantValueParser.java | 4 ++++ 11 files changed, 50 insertions(+), 7 deletions(-) diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hdgf/streams/CompressedStreamStore.java b/poi-scratchpad/src/main/java/org/apache/poi/hdgf/streams/CompressedStreamStore.java index 91ea0400e0..fc1057f839 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hdgf/streams/CompressedStreamStore.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hdgf/streams/CompressedStreamStore.java @@ -58,7 +58,7 @@ public final class CompressedStreamStore extends StreamStore { public static int getMaxRecordLength() { return MAX_RECORD_LENGTH; } - + /** * Creates a new compressed StreamStore, which will handle * the decompression. @@ -98,6 +98,10 @@ public final class CompressedStreamStore extends StreamStore { HDGFLZW lzw = new HDGFLZW(); byte[] decompressed = lzw.decompress(bais); + if (decompressed.length < 4) { + throw new IllegalArgumentException("Could not read enough data to decompress: " + decompressed.length); + } + // Split into header and contents byte[][] ret = new byte[2][]; ret[0] = new byte[4]; diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hpbf/model/qcbits/QCPLCBit.java b/poi-scratchpad/src/main/java/org/apache/poi/hpbf/model/qcbits/QCPLCBit.java index cd039cf36d..0879e394cf 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hpbf/model/qcbits/QCPLCBit.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hpbf/model/qcbits/QCPLCBit.java @@ -45,6 +45,9 @@ public abstract class QCPLCBit extends QCBit { // First four bytes are the number numberOfPLCs = (int)LittleEndian.getUInt(data, 0); + if (numberOfPLCs < 0) { + throw new IllegalArgumentException("Invalid number of PLCs: " + numberOfPLCs); + } // Next four bytes are the type typeOfPLCS = (int)LittleEndian.getUInt(data, 4); @@ -86,7 +89,7 @@ public abstract class QCPLCBit extends QCBit { this.plcValB = plcValB.clone(); } - + public static QCPLCBit createQCPLCBit(String thingType, String bitType, byte[] data) { // Grab the type @@ -217,7 +220,7 @@ public abstract class QCPLCBit extends QCBit { super(thingType, bitType, data); int cntPlcs = getNumberOfPLCs(); - + // How many hyperlinks do we really have? // (zero hyperlinks gets numberOfPLCs=1) hyperlinks = new String[data.length == 0x34 ? 0 : cntPlcs]; diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/Ffn.java b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/Ffn.java index bea8b2e581..7c30f2346a 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/Ffn.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/Ffn.java @@ -76,6 +76,11 @@ public final class Ffn { offsetTmp = offset - offsetTmp; _xszFfnLength = (this.getSize() - offsetTmp) / 2; + + if (_xszFfnLength < 0) { + throw new IllegalArgumentException("Had invalid computed size: " + _xszFfnLength + " with size " + getSize() + " and offsetTmp: " + offsetTmp); + } + _xszFfn = new char[_xszFfnLength]; for (int i = 0; i < _xszFfnLength; i++) { diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/LFOData.java b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/LFOData.java index 0e8cca03a4..726e67d7de 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/LFOData.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/LFOData.java @@ -43,8 +43,11 @@ public class LFOData _rgLfoLvl = new ListFormatOverrideLevel[0]; } - LFOData( byte[] buf, int startOffset, int cLfolvl ) - { + LFOData( byte[] buf, int startOffset, int cLfolvl ) { + if (cLfolvl < 0) { + throw new IllegalArgumentException("Cannot create LFOData with negative count"); + } + int offset = startOffset; _cp = LittleEndian.getInt( buf, offset ); diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/StyleSheet.java b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/StyleSheet.java index 18d703c835..c9b8880f28 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/StyleSheet.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/StyleSheet.java @@ -88,6 +88,10 @@ public final class StyleSheet { _stshif = new Stshif(tableStream, offset); + if (_stshif.getCstd() < 0) { + throw new IllegalArgumentException("Cannot create StyleSheet, invalid Cstd: " + _stshif.getCstd()); + } + // shall we discard cbLSD and mpstilsd? offset = startOffset + LittleEndianConsts.SHORT_SIZE + _cbStshi; diff --git a/poi/src/main/java/org/apache/poi/hssf/record/RecordFactory.java b/poi/src/main/java/org/apache/poi/hssf/record/RecordFactory.java index 86014c4d2b..0828e46537 100644 --- a/poi/src/main/java/org/apache/poi/hssf/record/RecordFactory.java +++ b/poi/src/main/java/org/apache/poi/hssf/record/RecordFactory.java @@ -22,6 +22,8 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import org.apache.poi.util.RecordFormatException; + /** * Title: Record Factory

* Description: Takes a stream and outputs an array of Record objects. @@ -103,6 +105,10 @@ public final class RecordFactory { * @return the equivalent array of {@link NumberRecord NumberRecords} */ public static NumberRecord[] convertRKRecords(MulRKRecord mrk) { + if (mrk.getNumColumns() < 0) { + throw new RecordFormatException("Cannot create RKRecords with negative number of columns: " + mrk.getNumColumns()); + } + NumberRecord[] mulRecs = new NumberRecord[mrk.getNumColumns()]; for (int k = 0; k < mrk.getNumColumns(); k++) { NumberRecord nr = new NumberRecord(); @@ -156,7 +162,7 @@ public final class RecordFactory { * * @exception org.apache.poi.util.RecordFormatException on error processing the InputStream */ - public static List createRecords(InputStream in) throws org.apache.poi.util.RecordFormatException { + public static List createRecords(InputStream in) throws RecordFormatException { List records = new ArrayList<>(NUM_RECORDS); 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 62535e50b8..b83100845b 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 @@ -330,7 +330,10 @@ public final class RecordInputStream implements LittleEndianInput { } else { nextRecord(); nextChunk = Math.min(available(),len); - assert(nextChunk > 0); + if (nextChunk <= 0) { + throw new RecordFormatException("Need to have a valid next chunk, but had: " + nextChunk + + " with len: " + len + " and available: " + available()); + } } } checkRecordPosition(nextChunk); diff --git a/poi/src/main/java/org/apache/poi/hssf/record/chart/ChartFRTInfoRecord.java b/poi/src/main/java/org/apache/poi/hssf/record/chart/ChartFRTInfoRecord.java index ed6e6c91d5..7d8dfd5355 100644 --- a/poi/src/main/java/org/apache/poi/hssf/record/chart/ChartFRTInfoRecord.java +++ b/poi/src/main/java/org/apache/poi/hssf/record/chart/ChartFRTInfoRecord.java @@ -78,6 +78,9 @@ public final class ChartFRTInfoRecord extends StandardRecord { verOriginator = in.readByte(); verWriter = in.readByte(); int cCFRTID = in.readShort(); + if (cCFRTID < 0) { + throw new IllegalArgumentException("Had negative CFRTID: " + cCFRTID); + } rgCFRTID = new CFRTID[cCFRTID]; for (int i = 0; i < cCFRTID; i++) { diff --git a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFRow.java b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFRow.java index 3487e31595..4a28ca4668 100644 --- a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFRow.java +++ b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFRow.java @@ -96,6 +96,10 @@ public final class HSSFRow implements Row, Comparable { row = record; setRowNum(record.getRowNumber()); + if (record.getLastCol() < 0 || INITIAL_CAPACITY < 0) { + throw new IllegalArgumentException("Had invalid column counts: " + record.getLastCol() + " and " + INITIAL_CAPACITY); + } + // Size the initial cell list such that a read only case won't waste // lots of memory, and a create/read followed by adding new cells can // add a bit without needing a resize diff --git a/poi/src/main/java/org/apache/poi/poifs/filesystem/BlockStore.java b/poi/src/main/java/org/apache/poi/poifs/filesystem/BlockStore.java index bdd016f860..178ff3aa8b 100644 --- a/poi/src/main/java/org/apache/poi/poifs/filesystem/BlockStore.java +++ b/poi/src/main/java/org/apache/poi/poifs/filesystem/BlockStore.java @@ -85,6 +85,10 @@ public abstract class BlockStore { protected class ChainLoopDetector { private final boolean[] used_blocks; protected ChainLoopDetector(long rawSize) { + if (rawSize < 0) { + throw new IllegalArgumentException("Cannot create a ChainLoopDetector with negative size, but had: " + rawSize); + } + int blkSize = getBlockStoreBlockSize(); int numBlocks = (int)(rawSize / blkSize); if ((rawSize % blkSize) != 0) { diff --git a/poi/src/main/java/org/apache/poi/ss/formula/constant/ConstantValueParser.java b/poi/src/main/java/org/apache/poi/ss/formula/constant/ConstantValueParser.java index e0e923b02e..9464ecbdb4 100644 --- a/poi/src/main/java/org/apache/poi/ss/formula/constant/ConstantValueParser.java +++ b/poi/src/main/java/org/apache/poi/ss/formula/constant/ConstantValueParser.java @@ -45,6 +45,10 @@ public final class ConstantValueParser { } public static Object[] parse(LittleEndianInput in, int nValues) { + if (nValues < 0) { + throw new IllegalArgumentException("Invalid number of values to parse: " + nValues); + } + Object[] result = new Object[nValues]; for (int i = 0; i < result.length; i++) { result[i] = readAConstantValue(in); -- cgit v1.2.3