From 9fa33b2b7e2fafb98fc9f5c784dd21487a14816a Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Mon, 13 Dec 2021 19:22:34 +0000 Subject: [PATCH] Fix issues found when fuzzing Apache POI via Jazzer Add some additional allocation limits to avoid OOM in some more cases with some broken input files git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1895922 13f79535-47bb-0310-9956-ffa450edef68 --- .../poi/openxml4j/util/ZipArchiveFakeEntry.java | 9 +++++++-- .../apache/poi/hdgf/pointers/PointerFactory.java | 13 ++++++++++++- .../org/apache/poi/hpbf/model/qcbits/QCPLCBit.java | 8 ++++++-- .../poi/hslf/usermodel/HSLFSlideShowImpl.java | 3 ++- .../poi/hslf/usermodel/HSLFTextParagraph.java | 8 +++++++- .../java/org/apache/poi/hwpf/model/PlexOfCps.java | 3 +++ .../poi/poifs/filesystem/POIFSFileSystem.java | 6 ++++++ 7 files changed, 43 insertions(+), 7 deletions(-) diff --git a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipArchiveFakeEntry.java b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipArchiveFakeEntry.java index 519323ec14..572f7bdc52 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipArchiveFakeEntry.java +++ b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/util/ZipArchiveFakeEntry.java @@ -35,7 +35,12 @@ import org.apache.poi.util.TempFile; * @see ZipInputStreamZipEntrySource#setThresholdBytesForTempFiles(int) */ /* package */ class ZipArchiveFakeEntry extends ZipArchiveEntry implements Closeable { - private static Logger LOG = LogManager.getLogger(ZipArchiveFakeEntry.class); + private static final Logger LOG = LogManager.getLogger(ZipArchiveFakeEntry.class); + + // how large a single entry in a zip-file should become at max + // can be overwritten via IOUtils.setByteArrayMaxOverride() + private static final int MAX_ENTRY_SIZE = 100_000_000; + private byte[] data; private File tempFile; private EncryptedTempData encryptedTempData; @@ -64,7 +69,7 @@ import org.apache.poi.util.TempFile; } // Grab the de-compressed contents for later - data = (entrySize == -1) ? IOUtils.toByteArray(inp) : IOUtils.toByteArray(inp, (int)entrySize); + data = (entrySize == -1) ? IOUtils.toByteArray(inp) : IOUtils.toByteArray(inp, (int)entrySize, MAX_ENTRY_SIZE); } } diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hdgf/pointers/PointerFactory.java b/poi-scratchpad/src/main/java/org/apache/poi/hdgf/pointers/PointerFactory.java index bb1d064369..504e242104 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hdgf/pointers/PointerFactory.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hdgf/pointers/PointerFactory.java @@ -18,6 +18,7 @@ package org.apache.poi.hdgf.pointers; import org.apache.poi.hdgf.streams.PointerContainingStream; +import org.apache.poi.util.IOUtils; import org.apache.poi.util.LittleEndian; /** @@ -25,10 +26,14 @@ import org.apache.poi.util.LittleEndian; * of the file */ public final class PointerFactory { - private int version; + private static final int MAX_NUMBER_OF_POINTERS = 100_000; + + private final int version; + public PointerFactory(int version) { this.version = version; } + public int getVersion() { return version; } /** @@ -71,6 +76,12 @@ public final class PointerFactory { // How much to skip for the num pointers + any extra data? int skip = parent.getPostNumPointersSkip(); + if (numPointers < 0) { + throw new IllegalArgumentException("Cannot create container pointers with negative count: " + numPointers); + } + + IOUtils.safelyAllocateCheck(numPointers, MAX_NUMBER_OF_POINTERS); + // Create int pos = numPointersOffset + skip; Pointer[] childPointers = new Pointer[numPointers]; 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 0879e394cf..af5a138e4f 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 @@ -17,6 +17,7 @@ package org.apache.poi.hpbf.model.qcbits; +import org.apache.poi.util.IOUtils; import org.apache.poi.util.LittleEndian; import org.apache.poi.util.StringUtil; @@ -26,8 +27,10 @@ import org.apache.poi.util.StringUtil; * format is determined by the type of the PLCs. */ public abstract class QCPLCBit extends QCBit { - private int numberOfPLCs; - private int typeOfPLCS; + private static final int MAX_NUMBER_OF_PLCS = 1000; + + private final int numberOfPLCs; + private final int typeOfPLCS; /** * The data which goes before the main PLC entries. * This is apparently always made up of 2 byte @@ -53,6 +56,7 @@ public abstract class QCPLCBit extends QCBit { typeOfPLCS = (int)LittleEndian.getUInt(data, 4); // Init the arrays that we can + IOUtils.safelyAllocateCheck(numberOfPLCs, MAX_NUMBER_OF_PLCS); plcValA = new long[numberOfPLCs]; plcValB = new long[numberOfPLCs]; } diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSlideShowImpl.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSlideShowImpl.java index 09dfffbcb9..931b2df2de 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSlideShowImpl.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFSlideShowImpl.java @@ -91,6 +91,7 @@ public final class HSLFSlideShowImpl extends POIDocument implements Closeable { //arbitrarily selected; may need to increase private static final int DEFAULT_MAX_RECORD_LENGTH = 200_000_000; + private static final int MAX_DOCUMENT_SIZE = 100_000_000; private static int MAX_RECORD_LENGTH = DEFAULT_MAX_RECORD_LENGTH; // Holds metadata on where things are in our document @@ -233,7 +234,7 @@ public final class HSLFSlideShowImpl extends POIDocument implements Closeable { // Grab the document stream int len = docProps.getSize(); try (InputStream is = dir.createDocumentInputStream(docProps)) { - _docstream = IOUtils.toByteArray(is, len); + _docstream = IOUtils.toByteArray(is, len, MAX_DOCUMENT_SIZE); } } diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFTextParagraph.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFTextParagraph.java index 98218e2d23..129c61c0f3 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFTextParagraph.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFTextParagraph.java @@ -86,6 +86,8 @@ import org.apache.poi.util.Units; public final class HSLFTextParagraph implements TextParagraph { private static final Logger LOG = LogManager.getLogger(HSLFTextParagraph.class); + private static final int MAX_NUMBER_OF_STYLES = 10_000; + // Note: These fields are protected to help with unit testing // Other classes shouldn't really go playing with them! private final TextHeaderAtom _headerAtom; @@ -1551,7 +1553,11 @@ public final class HSLFTextParagraph implements TextParagraph MAX_NUMBER_OF_STYLES) { + throw new IllegalStateException("Cannot process more than " + MAX_NUMBER_OF_STYLES + " styles, but had paragraph with " + ccStyle); + } + for (int ccRun = 0; ccRun < ccStyle;) { HSLFTextParagraph para = paragraphs.get(paraIdx); List runs = para.getTextRuns(); trun = runs.get(runIdx); diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/PlexOfCps.java b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/PlexOfCps.java index 6c8ee4dd46..fc2f85900c 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/PlexOfCps.java +++ b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/model/PlexOfCps.java @@ -37,6 +37,7 @@ public final class PlexOfCps { //arbitrarily selected; may need to increase private static final int MAX_RECORD_LENGTH = 10_485_760; + private static final int MAX_NUMBER_OF_PROPERTIES = 100_000; private int _iMac; private final int _cbStruct; @@ -57,6 +58,8 @@ public final class PlexOfCps { // Figure out the number we hold _iMac = (cb - 4) / (4 + cbStruct); + IOUtils.safelyAllocateCheck(_iMac, MAX_NUMBER_OF_PROPERTIES); + _cbStruct = cbStruct; _props = new ArrayList<>(_iMac); diff --git a/poi/src/main/java/org/apache/poi/poifs/filesystem/POIFSFileSystem.java b/poi/src/main/java/org/apache/poi/poifs/filesystem/POIFSFileSystem.java index 7ac3a2ccdd..eba62568b9 100644 --- a/poi/src/main/java/org/apache/poi/poifs/filesystem/POIFSFileSystem.java +++ b/poi/src/main/java/org/apache/poi/poifs/filesystem/POIFSFileSystem.java @@ -64,6 +64,8 @@ public class POIFSFileSystem extends BlockStore private static final int DEFAULT_MAX_RECORD_LENGTH = 100_000; private static int MAX_RECORD_LENGTH = DEFAULT_MAX_RECORD_LENGTH; + private static final int MAX_ALLOCATION_SIZE = 100_000_000; + private static final Logger LOG = LogManager.getLogger(POIFSFileSystem.class); /** @@ -334,6 +336,10 @@ public class POIFSFileSystem extends BlockStore if (maxSize > Integer.MAX_VALUE) { throw new IllegalArgumentException("Unable read a >2gb file via an InputStream"); } + + // don't allow huge allocations with invalid header-values + IOUtils.safelyAllocateCheck(maxSize, MAX_ALLOCATION_SIZE); + ByteBuffer data = ByteBuffer.allocate((int) maxSize); // Copy in the header -- 2.39.5