Avoid some NullPointerExceptions and ClassCastExceptions found when fuzzing Apache POI

This mostly only makes thrown exceptions a bit more consistent
or may allow some broken documents to be still read.

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1906322 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Dominik Stadler 2023-01-01 15:59:32 +00:00
parent 012bf1a99a
commit 5724a77cf2
16 changed files with 134 additions and 59 deletions

View File

@ -224,6 +224,9 @@ public class ReadOnlySharedStringsTable extends DefaultHandler implements Shared
@Override
public RichTextString getItemAt(int idx) {
if (strings == null || idx >= strings.size()) {
throw new IllegalStateException("Cannot get item at " + idx + " with strings: " + strings);
}
return new XSSFRichTextString(strings.get(idx));
}

View File

@ -300,6 +300,9 @@ public class XSSFReader {
* @throws IOException if there is an I/O issue reading the data
*/
protected SheetIterator(PackagePart wb) throws IOException, InvalidFormatException {
if (wb == null) {
throw new InvalidFormatException("Cannot create sheet-iterator with missing package part for workbook");
}
/*
* The order of sheets is defined by the order of CTSheet elements in workbook.xml

View File

@ -3751,6 +3751,10 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet, OoxmlSheetEx
}
protected void write(OutputStream out) throws IOException {
if (worksheet == null) {
throw new POIXMLException("Cannot write invalid sheet, internal data is missing");
}
boolean setToNull = false;
if(worksheet.sizeOfColsArray() == 1) {
CTCols col = worksheet.getColsArray(0);

View File

@ -59,6 +59,10 @@ public class XWPFComments extends POIXMLDocumentPart {
*/
public XWPFComments(POIXMLDocumentPart parent, PackagePart part) {
super(parent, part);
if (!(getParent() instanceof XWPFDocument)) {
throw new IllegalStateException("Parent is not a XWPFDocuemnt: " + getParent());
}
this.document = (XWPFDocument) getParent();
if (this.document == null) {

View File

@ -32,6 +32,11 @@ public class XWPFPicture {
public XWPFPicture(CTPicture ctPic, XWPFRun run) {
this.run = run;
this.ctPic = ctPic;
if (ctPic == null || ctPic.getNvPicPr() == null || ctPic.getNvPicPr().getCNvPr() == null) {
throw new IllegalArgumentException("Found missing data while reading picture data from " + ctPic);
}
description = ctPic.getNvPicPr().getCNvPr().getDescr();
}

View File

@ -82,8 +82,11 @@ public final class HDGFDiagram extends POIReadOnlyDocument {
trailerPointer = ptrFactory.createPointer(_docstream, 0x24);
// Now grab the trailer
trailer = (TrailerStream)
Stream.createStream(trailerPointer, _docstream, chunkFactory, ptrFactory);
Stream stream = Stream.createStream(trailerPointer, _docstream, chunkFactory, ptrFactory);
if (!(stream instanceof TrailerStream)) {
throw new IllegalStateException("Stream is not a TrailerStream: " + stream);
}
trailer = (TrailerStream)stream;
// Finally, find all our streams
trailer.findChildren(_docstream);

View File

@ -82,9 +82,8 @@ public class HSLFTabStopPropCollection extends TextProp {
leo.writeShort(count);
for (HSLFTabStop ts : tabStops) {
leo.writeShort(ts.getPosition());
leo.writeShort(ts.getType().nativeId);
leo.writeShort(ts.getType() == null ? TabStopType.LEFT.nativeId : ts.getType().nativeId);
}
}
@Override

View File

@ -128,7 +128,7 @@ public class SlideAtomLayout implements GenericRecord {
public void writeOut(OutputStream out) throws IOException {
// Write the geometry
byte[] buf = new byte[4];
LittleEndian.putInt(buf, 0, geometry.getNativeId());
LittleEndian.putInt(buf, 0, geometry == null ? 0 : geometry.getNativeId());
out.write(buf);
// Write the placeholder IDs
out.write(placeholderIDs);

View File

@ -318,10 +318,14 @@ public final class HSLFSlideShowImpl extends POIDocument implements Closeable {
recordMap.put(usrOffset, usr);
int psrOffset = usr.getPersistPointersOffset();
PersistPtrHolder ptr = (PersistPtrHolder) Record.buildRecordAtOffset(docstream, psrOffset);
if (ptr == null) {
Record record = Record.buildRecordAtOffset(docstream, psrOffset);
if (record == null) {
throw new CorruptPowerPointFileException("Powerpoint document is missing a PersistPtrHolder at " + psrOffset);
}
if (!(record instanceof PersistPtrHolder)) {
throw new CorruptPowerPointFileException("Record is not a PersistPtrHolder: " + record + " at " + psrOffset);
}
PersistPtrHolder ptr = (PersistPtrHolder) record;
recordMap.put(psrOffset, ptr);
for (Map.Entry<Integer, Integer> entry : ptr.getSlideLocationsLookup().entrySet()) {
@ -609,6 +613,9 @@ public final class HSLFSlideShowImpl extends POIDocument implements Closeable {
CountingOS cos = new CountingOS();
for (Record record : _records) {
// all top level records are position dependent
if (!(record instanceof PositionDependentRecord)) {
throw new CorruptPowerPointFileException("Record is not a position dependent record: " + record);
}
PositionDependentRecord pdr = (PositionDependentRecord) record;
int oldPos = pdr.getLastOnDiskOffset();
int newPos = cos.size();
@ -987,6 +994,10 @@ public final class HSLFSlideShowImpl extends POIDocument implements Closeable {
throw new CorruptPowerPointFileException("Document record is missing");
}
if (documentRecord.getPPDrawingGroup() == null) {
throw new CorruptPowerPointFileException("Drawing group is missing");
}
EscherContainerRecord blipStore;
EscherContainerRecord dggContainer = documentRecord.getPPDrawingGroup().getDggContainer();

View File

@ -24,6 +24,7 @@ import java.io.InputStream;
import java.text.SimpleDateFormat;
import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
import java.util.Iterator;
import java.util.Locale;
@ -127,7 +128,7 @@ public class OutlookTextExtractor implements POIOLE2TextExtractor {
// First try via the proper chunk
SimpleDateFormat f = new SimpleDateFormat("E, d MMM yyyy HH:mm:ss Z", Locale.ROOT);
f.setTimeZone(LocaleUtil.getUserTimeZone());
s.append("Date: ").append(f.format(msg.getMessageDate().getTime())).append("\n");
s.append("Date: ").append(f.format(msg.getMessageDate() == null ? new Date(0) : msg.getMessageDate().getTime())).append("\n");
} catch (ChunkNotFoundException e) {
try {
// Failing that try via the raw headers

View File

@ -142,7 +142,7 @@ public class Sttb
// cchData
size += LittleEndianConsts.SHORT_SIZE;
// data
size += 2 * data.length();
size += 2 * (data == null ? 0 : data.length());
}
}
else
@ -152,7 +152,7 @@ public class Sttb
// cchData
size += LittleEndianConsts.BYTE_SIZE;
// data
size += data.length();
size += (data == null ? 0 : data.length());
}
}

View File

@ -81,7 +81,12 @@ final class LinkTable {
int nCRNs = _countRecord.getNumberOfCRNs();
CRNRecord[] crns = new CRNRecord[nCRNs];
for (int i = 0; i < crns.length; i++) {
crns[i] = (CRNRecord) rs.getNext();
Record record = rs.getNext();
if (!(record instanceof CRNRecord)) {
throw new IllegalStateException("Record is not a CRNRecord: " +
(record == null ? "<null>" : record.getClass() + ": " + record) );
}
crns[i] = (CRNRecord) record;
}
_crns = crns;
}

View File

@ -44,7 +44,7 @@ public class AgileEncryptionVerifier extends EncryptionVerifier {
}
}
if (keyData == null) {
if (keyData == null || keyData.getHashSize() == null) {
throw new IllegalArgumentException("encryptedKey not set");
}

View File

@ -105,12 +105,16 @@ public final class PropertyTable implements BATManaged {
PropertyFactory.convertToProperties(data, _properties);
}
if (_properties.get(0) != null) {
populatePropertyTree((DirectoryProperty) _properties.get(0));
Property property = _properties.get(0);
if (property != null) {
if (property instanceof DirectoryProperty) {
populatePropertyTree((DirectoryProperty) property);
} else {
throw new IOException("Invalid format, cannot convert property " + property + " to DirectoryProperty");
}
}
}
/**
* Add a property to the list of properties we manage
*

View File

@ -371,6 +371,10 @@ public class DataFormatter {
}
private Format getFormat(double cellValue, int formatIndex, String formatStrIn, boolean use1904Windowing) {
if (formatStrIn == null) {
throw new IllegalArgumentException("Missing input format for value " + cellValue + " and index " + formatIndex);
}
checkForLocaleChange();
// Might be better to separate out the n p and z formats, falling back to p when n and z are not set.

View File

@ -22,8 +22,11 @@ import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedHashMap;
@ -32,6 +35,7 @@ import java.util.Map;
import org.apache.poi.hssf.HSSFTestDataSamples;
import org.apache.poi.hssf.record.BOFRecord;
import org.apache.poi.hssf.record.CRNCountRecord;
import org.apache.poi.hssf.record.CountryRecord;
import org.apache.poi.hssf.record.EOFRecord;
import org.apache.poi.hssf.record.ExternSheetRecord;
@ -39,11 +43,13 @@ import org.apache.poi.hssf.record.ExternalNameRecord;
import org.apache.poi.hssf.record.NameCommentRecord;
import org.apache.poi.hssf.record.NameRecord;
import org.apache.poi.hssf.record.Record;
import org.apache.poi.hssf.record.RecordInputStream;
import org.apache.poi.hssf.record.SSTRecord;
import org.apache.poi.hssf.record.SupBookRecord;
import org.apache.poi.hssf.usermodel.HSSFCell;
import org.apache.poi.hssf.usermodel.HSSFWorkbook;
import org.apache.poi.ss.formula.ptg.NameXPtg;
import org.apache.poi.util.LittleEndian;
import org.junit.jupiter.api.Test;
/**
@ -51,7 +57,7 @@ import org.junit.jupiter.api.Test;
*/
final class TestLinkTable {
/**
/*
* The example file attached to bugzilla 45046 is a clear example of Name records being present
* without an External Book (SupBook) record. Excel has no trouble reading this file.<br>
* TODO get OOO documentation updated to reflect this (that EXTERNALBOOK is optional).
@ -59,61 +65,64 @@ final class TestLinkTable {
* It's not clear what exact steps need to be taken in Excel to create such a workbook
*/
@Test
void testLinkTableWithoutExternalBookRecord_bug45046() {
void testLinkTableWithoutExternalBookRecord_bug45046() throws IOException {
// Bug 45046 b: DEFINEDNAME is part of LinkTable
HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("ex45046-21984.xls");
// some other sanity checks
assertEquals(3, wb.getNumberOfSheets());
String formula = wb.getSheetAt(0).getRow(4).getCell(13).getCellFormula();
try (HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("ex45046-21984.xls")) {
// some other sanity checks
assertEquals(3, wb.getNumberOfSheets());
String formula = wb.getSheetAt(0).getRow(4).getCell(13).getCellFormula();
// The reported symptom of this bugzilla is an earlier bug (already fixed)
// This is observable in version 3.0
assertNotEquals("ipcSummenproduktIntern($P5,N$6,$A$9,N$5)", formula);
// The reported symptom of this bugzilla is an earlier bug (already fixed)
// This is observable in version 3.0
assertNotEquals("ipcSummenproduktIntern($P5,N$6,$A$9,N$5)", formula);
assertEquals("ipcSummenproduktIntern($C5,N$2,$A$9,N$1)", formula);
assertEquals("ipcSummenproduktIntern($C5,N$2,$A$9,N$1)", formula);
}
}
@Test
void testMultipleExternSheetRecords_bug45698() {
void testMultipleExternSheetRecords_bug45698() throws IOException {
// Bug: Extern sheet is part of LinkTable
HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("ex45698-22488.xls");
// some other sanity checks
assertEquals(7, wb.getNumberOfSheets());
try (HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("ex45698-22488.xls")) {
// some other sanity checks
assertEquals(7, wb.getNumberOfSheets());
}
}
@Test
void testExtraSheetRefs_bug45978() {
HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("ex45978-extraLinkTableSheets.xls");
/*
ex45978-extraLinkTableSheets.xls is a cut-down version of attachment 22561.
The original file produces the same error.
void testExtraSheetRefs_bug45978() throws IOException {
try (HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("ex45978-extraLinkTableSheets.xls")) {
/*
ex45978-extraLinkTableSheets.xls is a cut-down version of attachment 22561.
The original file produces the same error.
This bug was caused by a combination of invalid sheet indexes in the EXTERNSHEET
record, and eager initialisation of the extern sheet references. Note - the workbook
has 2 sheets, but the EXTERNSHEET record refers to sheet indexes 0, 1 and 2.
This bug was caused by a combination of invalid sheet indexes in the EXTERNSHEET
record, and eager initialisation of the extern sheet references. Note - the workbook
has 2 sheets, but the EXTERNSHEET record refers to sheet indexes 0, 1 and 2.
Offset 0x3954 (14676)
recordid = 0x17, size = 32
[EXTERNSHEET]
numOfRefs = 5
refrec #0: extBook=0 firstSheet=0 lastSheet=0
refrec #1: extBook=1 firstSheet=2 lastSheet=2
refrec #2: extBook=2 firstSheet=1 lastSheet=1
refrec #3: extBook=0 firstSheet=-1 lastSheet=-1
refrec #4: extBook=0 firstSheet=1 lastSheet=1
[/EXTERNSHEET]
Offset 0x3954 (14676)
recordid = 0x17, size = 32
[EXTERNSHEET]
numOfRefs = 5
refrec #0: extBook=0 firstSheet=0 lastSheet=0
refrec #1: extBook=1 firstSheet=2 lastSheet=2
refrec #2: extBook=2 firstSheet=1 lastSheet=1
refrec #3: extBook=0 firstSheet=-1 lastSheet=-1
refrec #4: extBook=0 firstSheet=1 lastSheet=1
[/EXTERNSHEET]
As it turns out, the formula in question doesn't even use externSheetIndex #1 - it
uses #4, which resolves to sheetIndex 1 -> 'Data'.
As it turns out, the formula in question doesn't even use externSheetIndex #1 - it
uses #4, which resolves to sheetIndex 1 -> 'Data'.
It is not clear exactly what externSheetIndex #4 would refer to. Excel seems to
display such a formula as "''!$A2", but then complains of broken link errors.
*/
It is not clear exactly what externSheetIndex #4 would refer to. Excel seems to
display such a formula as "''!$A2", but then complains of broken link errors.
*/
HSSFCell cell = wb.getSheetAt(0).getRow(1).getCell(1);
// Bug: IndexOutOfBoundsException - Index: 2, Size: 2
String cellFormula = cell.getCellFormula();
assertEquals("Data!$A2", cellFormula);
HSSFCell cell = wb.getSheetAt(0).getRow(1).getCell(1);
// Bug: IndexOutOfBoundsException - Index: 2, Size: 2
String cellFormula = cell.getCellFormula();
assertEquals("Data!$A2", cellFormula);
}
}
/**
@ -122,7 +131,6 @@ final class TestLinkTable {
*/
@Test
void testMissingExternSheetRecord_bug47001b() {
Record[] recs = {
SupBookRecord.createAddInFunctions(),
new SSTRecord(),
@ -136,8 +144,30 @@ final class TestLinkTable {
}
@Test
void testNameCommentRecordBetweenNameRecords() {
void testCRNCountRecordInvalid() {
byte[] data = new byte[22];
LittleEndian.putShort(data, 0, CRNCountRecord.sid);
LittleEndian.putShort(data, 2, (short)18);
LittleEndian.putShort(data, 4, (short)55);
LittleEndian.putInt(data, 6, 56);
RecordInputStream in = new RecordInputStream(new ByteArrayInputStream(data));
in.nextRecord();
Record[] recs = {
SupBookRecord.createAddInFunctions(),
new CRNCountRecord(in),
new SSTRecord(),
};
List<org.apache.poi.hssf.record.Record> recList = Arrays.asList(recs);
WorkbookRecordList wrl = new WorkbookRecordList();
assertThrows(IllegalStateException.class,
() -> new LinkTable(recList, 0, wrl, Collections.emptyMap()));
}
@Test
void testNameCommentRecordBetweenNameRecords() {
final Record[] recs = {
new NameRecord(),
new NameCommentRecord("name1", "comment1"),
@ -251,6 +281,5 @@ final class TestLinkTable {
assertEquals(0, tbl.resolveNameXIx(namex2.getSheetRefIndex(), namex2.getNameIndex()));
assertEquals("ISEVEN", tbl.resolveNameXText(namex2.getSheetRefIndex(), namex2.getNameIndex(), null));
}
}