Skip to content

Commit

Permalink
Bug 66425: Avoid exceptions found via poi-fuzz
Browse files Browse the repository at this point in the history
We try to avoid throwing NullPointerException, ClassCastExceptions and StackOverflowException, but it was possible
to trigger them

Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=61562
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=62068

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1912383 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
centic9 committed Sep 18, 2023
1 parent 836512c commit 88bbfbb
Show file tree
Hide file tree
Showing 10 changed files with 35 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,9 @@ public ParentPartConstructor getParentPartConstructor() {
* @since 3.16-beta3
*/
public InputStream getContents(PackagePart corePart) throws IOException, InvalidFormatException {
if (corePart == null) {
throw new IllegalArgumentException("Core-Part cannot be empty");
}
PackageRelationshipCollection prc =
corePart.getRelationshipsByType(getRelation());
Iterator<PackageRelationship> it = prc.iterator();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ public POIXMLTextExtractor create(OPCPackage pkg) throws IOException {

// Grab the core document part, and try to identify from that
final PackagePart corePart = pkg.getPart(core.getRelationship(0));
final String contentType = corePart.getContentType();
final String contentType = corePart == null ? null : corePart.getContentType();

// Is it XSSF?
for (XSSFRelation rel : XSSFExcelExtractor.SUPPORTED_TYPES) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ Licensed to the Apache Software Foundation (ASF) under one or more
import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;

import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.transform.OutputKeys;
import javax.xml.transform.Transformer;
import javax.xml.transform.dom.DOMSource;
Expand Down Expand Up @@ -53,10 +51,11 @@ Licensed to the Apache Software Foundation (ASF) under one or more
import org.w3c.dom.Element;

@Beta
public class WordToTextConverter extends AbstractWordConverter
{
public class WordToTextConverter extends AbstractWordConverter {
private static final Logger LOG = LogManager.getLogger(WordToTextConverter.class);

private static final int MAX_NESTED_CHILD_NODES = 500;

public static String getText( DirectoryNode root ) throws Exception
{
final HWPFDocumentCore wordDocument = AbstractWordUtils.loadDoc( root );
Expand Down Expand Up @@ -109,7 +108,7 @@ public static void main( String[] args ) throws Exception {
serializer.transform( domSource, streamResult );
}

private static Document process( File docFile ) throws IOException, ParserConfigurationException {
private static Document process( File docFile ) throws IOException {
try (final HWPFDocumentCore wordDocument = AbstractWordUtils.loadDoc( docFile )) {
WordToTextConverter wordToTextConverter = new WordToTextConverter(
XMLHelper.newDocumentBuilder().newDocument());
Expand All @@ -118,7 +117,7 @@ private static Document process( File docFile ) throws IOException, ParserConfig
}
}

private AtomicInteger noteCounters = new AtomicInteger( 1 );
private final AtomicInteger noteCounters = new AtomicInteger( 1 );

private Element notes;

Expand All @@ -130,11 +129,8 @@ private static Document process( File docFile ) throws IOException, ParserConfig
* Creates new instance of {@link WordToTextConverter}. Can be used for
* output several {@link HWPFDocument}s into single text document.
*
* @throws ParserConfigurationException
* if an internal {@link DocumentBuilder} cannot be created
*/
public WordToTextConverter() throws ParserConfigurationException
{
public WordToTextConverter() {
this.textDocumentFacade = new TextDocumentFacade(
XMLHelper.newDocumentBuilder().newDocument() );
}
Expand Down Expand Up @@ -312,6 +308,12 @@ private void processNote( HWPFDocument wordDocument, Element block, Range noteTe
Element note = textDocumentFacade.createBlock();
notes.appendChild( note );

// avoid StackOverflowException with very deeply nested files (mostly synthetic test files via fuzzing)
// if this limit is reached in real-word documents, we can make this configurable
if (note.getParentNode() != null && note.getParentNode().getChildNodes().getLength() > MAX_NESTED_CHILD_NODES) {
throw new IllegalStateException("Had more than the limit of " + MAX_NESTED_CHILD_NODES + " nested child notes");
}

note.appendChild( textDocumentFacade.createText( "^" + noteIndex
+ "\t " ) );
processCharacters( wordDocument, Integer.MIN_VALUE, noteTextRange, note );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more

import org.apache.poi.POIDataSamples;
import org.apache.poi.hslf.exceptions.EncryptedPowerPointFileException;
import org.apache.poi.hslf.exceptions.HSLFException;
import org.apache.poi.hslf.exceptions.OldPowerPointFormatException;
import org.apache.poi.util.IOUtils;
import org.apache.commons.io.output.NullPrintStream;
Expand Down Expand Up @@ -65,6 +66,7 @@ public abstract class BaseTestPPTIterating {
EXCLUDED.put("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6710128412590080.ppt", RuntimeException.class);
EXCLUDED.put("clusterfuzz-testcase-minimized-POIFuzzer-5429732352851968.ppt", FileNotFoundException.class);
EXCLUDED.put("clusterfuzz-testcase-minimized-POIFuzzer-5681320547975168.ppt", FileNotFoundException.class);
EXCLUDED.put("clusterfuzz-testcase-minimized-POIHSLFFuzzer-5962760801091584.ppt", RuntimeException.class);
}

public static Stream<Arguments> files() {
Expand Down
18 changes: 17 additions & 1 deletion poi/src/main/java/org/apache/poi/ddf/EscherContainerRecord.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ public final class EscherContainerRecord extends EscherRecord implements Iterabl

private static final Logger LOGGER = LogManager.getLogger(EscherContainerRecord.class);

private static final int MAX_NESTED_CHILD_NODES = 1000;

/**
* in case if document contains any charts we have such document structure:
* BOF
Expand Down Expand Up @@ -86,12 +88,26 @@ public EscherContainerRecord(EscherContainerRecord other) {

@Override
public int fillFields(byte[] data, int pOffset, EscherRecordFactory recordFactory) {
return fillFields(data, pOffset, recordFactory, 0);
}

private int fillFields(byte[] data, int pOffset, EscherRecordFactory recordFactory, int nesting) {
if (nesting > MAX_NESTED_CHILD_NODES) {
throw new IllegalStateException("Had more than the limit of " + MAX_NESTED_CHILD_NODES + " nested child notes");
}
int bytesRemaining = readHeader(data, pOffset);
int bytesWritten = 8;
int offset = pOffset + 8;
while (bytesRemaining > 0 && offset < data.length) {
EscherRecord child = recordFactory.createRecord(data, offset);
int childBytesWritten = child.fillFields(data, offset, recordFactory);

final int childBytesWritten;
if (child instanceof EscherContainerRecord) {
childBytesWritten = ((EscherContainerRecord)child).fillFields(data, offset, recordFactory, nesting + 1);
} else {
childBytesWritten = child.fillFields(data, offset, recordFactory);
}

bytesWritten += childBytesWritten;
offset += childBytesWritten;
bytesRemaining -= childBytesWritten;
Expand Down
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file modified test-data/spreadsheet/stress.xls
Binary file not shown.

0 comments on commit 88bbfbb

Please sign in to comment.