From 4df10cf9dde47a2f73a18197d916bb6d2d60cdf4 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Fri, 29 Jul 2022 17:07:15 +0000 Subject: [PATCH] Fix issues found when fuzzing Apache POI via Jazzer Replace RuntimeException with a more specific types git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1903104 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/poi/xwpf/usermodel/XWPFSettings.java | 2 +- .../org/apache/poi/hssf/model/RecordOrderer.java | 16 ++++++++-------- .../org/apache/poi/hssf/model/RecordStream.java | 3 ++- .../apache/poi/hssf/model/RowBlocksReader.java | 4 ++-- .../apache/poi/hssf/record/ColumnInfoRecord.java | 2 +- .../apache/poi/hssf/record/SupBookRecord.java | 4 ++-- .../record/aggregates/RowRecordsAggregate.java | 10 +++++----- .../record/aggregates/SharedValueManager.java | 5 +++-- .../poi/hssf/util/CellRangeAddress8Bit.java | 2 +- .../standard/StandardEncryptionVerifier.java | 4 ++-- .../poifs/filesystem/DocumentInputStream.java | 4 ++-- .../ss/formula/constant/ConstantValueParser.java | 4 ++-- .../java/org/apache/poi/ss/formula/ptg/Ptg.java | 8 ++++---- .../org/apache/poi/ss/util/CellRangeAddress.java | 2 +- .../util/LittleEndianByteArrayInputStream.java | 2 +- .../org/apache/poi/hssf/usermodel/TestBugs.java | 2 +- 16 files changed, 38 insertions(+), 36 deletions(-) diff --git a/poi-ooxml/src/main/java/org/apache/poi/xwpf/usermodel/XWPFSettings.java b/poi-ooxml/src/main/java/org/apache/poi/xwpf/usermodel/XWPFSettings.java index 407af7232f..5c9098fb53 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/xwpf/usermodel/XWPFSettings.java +++ b/poi-ooxml/src/main/java/org/apache/poi/xwpf/usermodel/XWPFSettings.java @@ -416,7 +416,7 @@ public class XWPFSettings extends POIXMLDocumentPart { try { ctSettings = SettingsDocument.Factory.parse(inputStream, DEFAULT_XML_OPTIONS).getSettings(); } catch (Exception e) { - throw new RuntimeException(e); + throw new IllegalArgumentException("Failed to read data from input-stream", e); } } diff --git a/poi/src/main/java/org/apache/poi/hssf/model/RecordOrderer.java b/poi/src/main/java/org/apache/poi/hssf/model/RecordOrderer.java index 3d9eaa2669..c627561b61 100644 --- a/poi/src/main/java/org/apache/poi/hssf/model/RecordOrderer.java +++ b/poi/src/main/java/org/apache/poi/hssf/model/RecordOrderer.java @@ -112,7 +112,7 @@ final class RecordOrderer { if (recClass == WorksheetProtectionBlock.class) { return getWorksheetProtectionBlockInsertPos(records); } - throw new RuntimeException("Unexpected record class (" + recClass.getName() + ")"); + throw new IllegalArgumentException("Unexpected record class (" + recClass.getName() + ")"); } /** @@ -182,7 +182,7 @@ final class RecordOrderer { return i+1; } } - throw new RuntimeException("Did not find insert point for GUTS"); + throw new IllegalArgumentException("Did not find insert point for GUTS"); } private static boolean isPageBreakPriorRecord(Object rb) { if (rb instanceof Record) { @@ -242,7 +242,7 @@ final class RecordOrderer { // DataValidityTable } } - throw new RuntimeException("Did not find Window2 record"); + throw new IllegalArgumentException("Did not find Window2 record"); } private static int findInsertPosForNewMergedRecordTable(List records) { @@ -265,7 +265,7 @@ final class RecordOrderer { return i + 1; } } - throw new RuntimeException("Did not find Window2 record"); + throw new IllegalArgumentException("Did not find Window2 record"); } @@ -332,7 +332,7 @@ final class RecordOrderer { // ConditionalFormattingTable case HyperlinkRecord.sid: case UnknownRecord.QUICKTIP_0800: - // name of a VBA module + // name of a VBA module case UnknownRecord.CODENAME_1BA: return true; } @@ -361,7 +361,7 @@ final class RecordOrderer { } } // worksheet stream is seriously broken - throw new RuntimeException("DimensionsRecord not found"); + throw new IllegalArgumentException("DimensionsRecord not found"); } private static int getGutsRecordInsertPos(List records) { @@ -374,7 +374,7 @@ final class RecordOrderer { return i+1; } } - throw new RuntimeException("Did not find insert point for GUTS"); + throw new IllegalArgumentException("Did not find insert point for GUTS"); } private static boolean isGutsPriorRecord(RecordBase rb) { @@ -427,7 +427,7 @@ final class RecordOrderer { return true; case EOFRecord.sid: // WINDOW2 should always be present, so shouldn't have got this far - throw new RuntimeException("Found EOFRecord before WindowTwoRecord was encountered"); + throw new IllegalArgumentException("Found EOFRecord before WindowTwoRecord was encountered"); } return PageSettingsBlock.isComponentRecord(sid); } diff --git a/poi/src/main/java/org/apache/poi/hssf/model/RecordStream.java b/poi/src/main/java/org/apache/poi/hssf/model/RecordStream.java index e0a546cbbb..af57a55716 100644 --- a/poi/src/main/java/org/apache/poi/hssf/model/RecordStream.java +++ b/poi/src/main/java/org/apache/poi/hssf/model/RecordStream.java @@ -18,6 +18,7 @@ package org.apache.poi.hssf.model; import java.util.List; +import java.util.NoSuchElementException; import org.apache.poi.hssf.record.Record; /** @@ -54,7 +55,7 @@ public final class RecordStream { public Record getNext() { if(!hasNext()) { - throw new RuntimeException("Attempt to read past end of record stream"); + throw new NoSuchElementException("Attempt to read past end of record stream"); } _countRead ++; return _list.get(_nextIndex++); diff --git a/poi/src/main/java/org/apache/poi/hssf/model/RowBlocksReader.java b/poi/src/main/java/org/apache/poi/hssf/model/RowBlocksReader.java index 946d2c610f..b348f28a65 100644 --- a/poi/src/main/java/org/apache/poi/hssf/model/RowBlocksReader.java +++ b/poi/src/main/java/org/apache/poi/hssf/model/RowBlocksReader.java @@ -61,7 +61,7 @@ public final class RowBlocksReader { // records from a subsequent sheet. For example, if SharedFormulaRecords // are taken from the wrong sheet, this could cause bug 44449. if (!rs.hasNext()) { - throw new RuntimeException("Failed to find end of row/cell records"); + throw new IllegalStateException("Failed to find end of row/cell records"); } Record rec = rs.getNext(); @@ -70,7 +70,7 @@ public final class RowBlocksReader { case MergeCellsRecord.sid: dest = mergeCellRecords; break; case SharedFormulaRecord.sid: dest = shFrmRecords; if (!(prevRec instanceof FormulaRecord)) { - throw new RuntimeException("Shared formula record should follow a FormulaRecord"); + throw new IllegalStateException("Shared formula record should follow a FormulaRecord, but had " + prevRec); } FormulaRecord fr = (FormulaRecord)prevRec; firstCellRefs.add(new CellReference(fr.getRow(), fr.getColumn())); diff --git a/poi/src/main/java/org/apache/poi/hssf/record/ColumnInfoRecord.java b/poi/src/main/java/org/apache/poi/hssf/record/ColumnInfoRecord.java index 6cbdc79691..f9e1a330ff 100644 --- a/poi/src/main/java/org/apache/poi/hssf/record/ColumnInfoRecord.java +++ b/poi/src/main/java/org/apache/poi/hssf/record/ColumnInfoRecord.java @@ -85,7 +85,7 @@ public final class ColumnInfoRecord extends StandardRecord { field_6_reserved = 0; break; default: - throw new RuntimeException("Unusual record size remaining=(" + in.remaining() + ")"); + throw new IllegalArgumentException("Unusual record size remaining=(" + in.remaining() + ")"); } } diff --git a/poi/src/main/java/org/apache/poi/hssf/record/SupBookRecord.java b/poi/src/main/java/org/apache/poi/hssf/record/SupBookRecord.java index 240c7b4c35..bdf88e1cc3 100644 --- a/poi/src/main/java/org/apache/poi/hssf/record/SupBookRecord.java +++ b/poi/src/main/java/org/apache/poi/hssf/record/SupBookRecord.java @@ -132,11 +132,11 @@ public final class SupBookRecord extends StandardRecord { // 5.38.3 'Add-In Functions' _isAddInFunctions = true; if(field_1_number_of_sheets != 1) { - throw new RuntimeException("Expected 0x0001 for number of sheets field in 'Add-In Functions' but got (" + throw new IllegalArgumentException("Expected 0x0001 for number of sheets field in 'Add-In Functions' but got (" + field_1_number_of_sheets + ")"); } } else { - throw new RuntimeException("invalid EXTERNALBOOK code (" + throw new IllegalArgumentException("invalid EXTERNALBOOK code (" + Integer.toHexString(nextShort) + ")"); } } diff --git a/poi/src/main/java/org/apache/poi/hssf/record/aggregates/RowRecordsAggregate.java b/poi/src/main/java/org/apache/poi/hssf/record/aggregates/RowRecordsAggregate.java index d37c911c72..2f6a8bd2e5 100644 --- a/poi/src/main/java/org/apache/poi/hssf/record/aggregates/RowRecordsAggregate.java +++ b/poi/src/main/java/org/apache/poi/hssf/record/aggregates/RowRecordsAggregate.java @@ -108,7 +108,7 @@ public final class RowRecordsAggregate extends RecordAggregate { continue; } if (!(rec instanceof CellValueRecordInterface)) { - throw new RuntimeException("Unexpected record type (" + rec.getClass().getName() + ")"); + throw new IllegalArgumentException("Unexpected record type (" + rec.getClass().getName() + ")"); } _valuesAgg.construct((CellValueRecordInterface)rec, rs, svm); } @@ -145,11 +145,11 @@ public final class RowRecordsAggregate extends RecordAggregate { _valuesAgg.removeAllCellsValuesForRow(rowIndex); RowRecord rr = _rowRecords.remove(rowIndex); if (rr == null) { - throw new RuntimeException("Invalid row index (" + rowIndex + ")"); + throw new IllegalArgumentException("Invalid row index (" + rowIndex + ")"); } if (row != rr) { _rowRecords.put(rowIndex, rr); - throw new RuntimeException("Attempt to remove row that does not belong to this sheet"); + throw new IllegalArgumentException("Attempt to remove row that does not belong to this sheet"); } // Clear the cached values @@ -215,7 +215,7 @@ public final class RowRecordsAggregate extends RecordAggregate { try { return _rowRecordValues[startIndex].getRowNumber(); } catch(ArrayIndexOutOfBoundsException e) { - throw new RuntimeException("Did not find start row for block " + block); + throw new IllegalArgumentException("Did not find start row for block " + block); } } @@ -232,7 +232,7 @@ public final class RowRecordsAggregate extends RecordAggregate { try { return _rowRecordValues[endIndex].getRowNumber(); } catch(ArrayIndexOutOfBoundsException e) { - throw new RuntimeException("Did not find end row for block " + block); + throw new IllegalArgumentException("Did not find end row for block " + block); } } diff --git a/poi/src/main/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java b/poi/src/main/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java index 45e3c12c44..0a22a74f67 100644 --- a/poi/src/main/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java +++ b/poi/src/main/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java @@ -74,7 +74,8 @@ public final class SharedValueManager { } } if (_numberOfFormulas >= _frAggs.length) { - throw new RuntimeException("Too many formula records for shared formula group"); + throw new IllegalStateException("Too many formula records for shared formula group: " + _numberOfFormulas + + ", expecting less than " + _frAggs.length); } _frAggs[_numberOfFormulas++] = agg; } @@ -153,7 +154,7 @@ public final class SharedValueManager { public SharedFormulaRecord linkSharedFormulaRecord(CellReference firstCell, FormulaRecordAggregate agg) { SharedFormulaGroup result = findFormulaGroupForCell(firstCell); if(null == result) { - throw new RuntimeException("Failed to find a matching shared formula record"); + throw new IllegalArgumentException("Failed to find a matching shared formula record for cell: " + firstCell); } result.add(agg); return result.getSFR(); diff --git a/poi/src/main/java/org/apache/poi/hssf/util/CellRangeAddress8Bit.java b/poi/src/main/java/org/apache/poi/hssf/util/CellRangeAddress8Bit.java index a3d53894c0..b43fdffd58 100644 --- a/poi/src/main/java/org/apache/poi/hssf/util/CellRangeAddress8Bit.java +++ b/poi/src/main/java/org/apache/poi/hssf/util/CellRangeAddress8Bit.java @@ -41,7 +41,7 @@ public final class CellRangeAddress8Bit extends CellRangeAddressBase { private static int readUShortAndCheck(LittleEndianInput in) { if (in.available() < ENCODED_SIZE) { // Ran out of data - throw new RuntimeException("Ran out of data reading CellRangeAddress"); + throw new IllegalArgumentException("Ran out of data reading CellRangeAddress, available: " + in.available()); } return in.readUShort(); } diff --git a/poi/src/main/java/org/apache/poi/poifs/crypt/standard/StandardEncryptionVerifier.java b/poi/src/main/java/org/apache/poi/poifs/crypt/standard/StandardEncryptionVerifier.java index d30fe998b2..52f285eee3 100644 --- a/poi/src/main/java/org/apache/poi/poifs/crypt/standard/StandardEncryptionVerifier.java +++ b/poi/src/main/java/org/apache/poi/poifs/crypt/standard/StandardEncryptionVerifier.java @@ -34,8 +34,8 @@ public class StandardEncryptionVerifier extends EncryptionVerifier implements En protected StandardEncryptionVerifier(LittleEndianInput is, StandardEncryptionHeader header) { int saltSize = is.readInt(); - if (saltSize!=16) { - throw new RuntimeException("Salt size != 16 !?"); + if (saltSize != 16) { + throw new IllegalArgumentException("Salt size != 16: " + saltSize); } byte[] salt = new byte[16]; diff --git a/poi/src/main/java/org/apache/poi/poifs/filesystem/DocumentInputStream.java b/poi/src/main/java/org/apache/poi/poifs/filesystem/DocumentInputStream.java index 42add5e342..a4e76fab89 100644 --- a/poi/src/main/java/org/apache/poi/poifs/filesystem/DocumentInputStream.java +++ b/poi/src/main/java/org/apache/poi/poifs/filesystem/DocumentInputStream.java @@ -263,7 +263,7 @@ public final class DocumentInputStream extends InputStream implements LittleEndi throw new IllegalStateException("cannot perform requested operation on a closed stream"); } if (requestedSize > _document_size - _current_offset) { - throw new RuntimeException("Buffer underrun - requested " + requestedSize + throw new IllegalStateException("Buffer underrun - requested " + requestedSize + " bytes but " + (_document_size - _current_offset) + " was available"); } } @@ -276,7 +276,7 @@ public final class DocumentInputStream extends InputStream implements LittleEndi @Override public void readFully(byte[] buf, int off, int len) { if (len < 0) { - throw new RuntimeException("Can't read negative number of bytes"); + throw new IllegalArgumentException("Can't read negative number of bytes, but had: " + len); } checkAvaliable(len); 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 9464ecbdb4..432eb3b897 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 @@ -75,7 +75,7 @@ public final class ConstantValueParser { in.readInt(); return ErrorConstant.valueOf(errCode); } - throw new RuntimeException("Unknown grbit value (" + grbit + ")"); + throw new IllegalArgumentException("Unknown grbit value (" + grbit + ")"); } private static Object readBoolean(LittleEndianInput in) { @@ -87,7 +87,7 @@ public final class ConstantValueParser { return Boolean.TRUE; } // Don't tolerate unusual boolean encoded values (unless it becomes evident that they occur) - throw new RuntimeException("unexpected boolean encoding (" + val + ")"); + throw new IllegalArgumentException("unexpected boolean encoding (" + val + ")"); } public static int getEncodedSize(Object[] values) { diff --git a/poi/src/main/java/org/apache/poi/ss/formula/ptg/Ptg.java b/poi/src/main/java/org/apache/poi/ss/formula/ptg/Ptg.java index 52c5daf5e8..5f53737c94 100644 --- a/poi/src/main/java/org/apache/poi/ss/formula/ptg/Ptg.java +++ b/poi/src/main/java/org/apache/poi/ss/formula/ptg/Ptg.java @@ -71,7 +71,7 @@ public abstract class Ptg implements Duplicatable, GenericRecord { temp.add(ptg); } if(pos != size) { - throw new RuntimeException("Ptg array size mismatch"); + throw new IllegalArgumentException("Ptg array size mismatch"); } if (hasArrayPtgs) { Ptg[] result = toPtgArray(temp); @@ -167,7 +167,7 @@ public abstract class Ptg implements Duplicatable, GenericRecord { case IntPtg.sid: return new IntPtg(in); // 0x1e case NumberPtg.sid: return new NumberPtg(in); // 0x1f } - throw new RuntimeException("Unexpected base token id (" + id + ")"); + throw new IllegalArgumentException("Unexpected base token id (" + id + ")"); } private static Ptg[] toPtgArray(List l) { @@ -254,7 +254,7 @@ public abstract class Ptg implements Duplicatable, GenericRecord { public final void setClass(byte thePtgClass) { if (isBaseToken()) { - throw new RuntimeException("setClass should not be called on a base token"); + throw new IllegalStateException("setClass should not be called on a base token"); } ptgClass = thePtgClass; } @@ -279,7 +279,7 @@ public abstract class Ptg implements Duplicatable, GenericRecord { case Ptg.CLASS_VALUE: return 'V'; case Ptg.CLASS_ARRAY: return 'A'; } - throw new RuntimeException("Unknown operand class (" + ptgClass + ")"); + throw new IllegalArgumentException("Unknown operand class (" + ptgClass + ")"); } public abstract byte getDefaultOperandClass(); diff --git a/poi/src/main/java/org/apache/poi/ss/util/CellRangeAddress.java b/poi/src/main/java/org/apache/poi/ss/util/CellRangeAddress.java index 4d4d8af62a..91a60273a4 100644 --- a/poi/src/main/java/org/apache/poi/ss/util/CellRangeAddress.java +++ b/poi/src/main/java/org/apache/poi/ss/util/CellRangeAddress.java @@ -64,7 +64,7 @@ public class CellRangeAddress extends CellRangeAddressBase { private static int readUShortAndCheck(RecordInputStream in) { if (in.remaining() < ENCODED_SIZE) { // Ran out of data - throw new RuntimeException("Ran out of data reading CellRangeAddress"); + throw new IllegalArgumentException("Ran out of data reading CellRangeAddress"); } return in.readUShort(); } diff --git a/poi/src/main/java/org/apache/poi/util/LittleEndianByteArrayInputStream.java b/poi/src/main/java/org/apache/poi/util/LittleEndianByteArrayInputStream.java index 69b5678ee0..e5c0b259a0 100644 --- a/poi/src/main/java/org/apache/poi/util/LittleEndianByteArrayInputStream.java +++ b/poi/src/main/java/org/apache/poi/util/LittleEndianByteArrayInputStream.java @@ -76,7 +76,7 @@ public class LittleEndianByteArrayInputStream extends ByteArrayInputStream imple protected void checkPosition(int i) { if (i > count - pos) { - throw new RuntimeException("Buffer overrun, having " + count + " bytes in the stream and position is at " + pos + + throw new IllegalStateException("Buffer overrun, having " + count + " bytes in the stream and position is at " + pos + ", but trying to increment position by " + i); } } diff --git a/poi/src/test/java/org/apache/poi/hssf/usermodel/TestBugs.java b/poi/src/test/java/org/apache/poi/hssf/usermodel/TestBugs.java index 4f216c6848..4a11860161 100644 --- a/poi/src/test/java/org/apache/poi/hssf/usermodel/TestBugs.java +++ b/poi/src/test/java/org/apache/poi/hssf/usermodel/TestBugs.java @@ -2457,7 +2457,7 @@ final class TestBugs extends BaseTestBugzillaIssues { RuntimeException.class, () -> new PropertySet(new DocumentInputStream(entry)) ); - assertEquals("Can't read negative number of bytes", ex.getMessage()); + assertEquals("Can't read negative number of bytes, but had: -218103608", ex.getMessage()); } }