Bug 66425: Avoid exceptions found via poi-fuzz

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=62530 and https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=62491 git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1912433 13f79535-47bb-0310-9956-ffa450edef68
apache · Sep 20, 2023 · ce91967 · ce91967
1 parent 857b96b
commit ce91967
Show file tree

Hide file tree

Showing 12 changed files with 35 additions and 8 deletions.
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hwpf/converter/WordToTextConverter.java b/poi-scratchpad/src/main/java/org/apache/poi/hwpf/converter/WordToTextConverter.java
@@ -54,7 +54,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more
 public class WordToTextConverter extends AbstractWordConverter {
     private static final Logger LOG = LogManager.getLogger(WordToTextConverter.class);
 
-    private static final int MAX_NESTED_CHILD_NODES = 400;
+    private static final int MAX_NESTED_CHILD_NODES = 300;
 
     public static String getText( DirectoryNode root ) throws Exception
     {

diff --git a/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/BaseTestPPTIterating.java b/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/BaseTestPPTIterating.java
@@ -35,7 +35,6 @@ 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;
@@ -67,6 +66,7 @@ public abstract class BaseTestPPTIterating {
         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);
+        EXCLUDED.put("clusterfuzz-testcase-minimized-POIHSLFFuzzer-5231088823566336.ppt", FileNotFoundException.class);
     }
 
     public static Stream<Arguments> files() {

diff --git a/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestPPTXMLDump.java b/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestPPTXMLDump.java
@@ -65,7 +65,8 @@ void runOneFile(File pFile) throws Exception {
 
         // work around two files which works here but not in other tests
         if (pFile.getName().equals("clusterfuzz-testcase-minimized-POIFuzzer-5429732352851968.ppt") ||
-                pFile.getName().equals("clusterfuzz-testcase-minimized-POIFuzzer-5681320547975168.ppt")) {
+                pFile.getName().equals("clusterfuzz-testcase-minimized-POIFuzzer-5681320547975168.ppt") ||
+                pFile.getName().equals("clusterfuzz-testcase-minimized-POIHSLFFuzzer-5231088823566336.ppt")) {
             throw new FileNotFoundException();
         }
     }

diff --git a/poi/src/main/java/org/apache/poi/hssf/record/CFRule12Record.java b/poi/src/main/java/org/apache/poi/hssf/record/CFRule12Record.java
@@ -408,7 +408,9 @@ public void serialize(LittleEndianOutput out) {
         out.writeShort(priority);
         out.writeShort(template_type);
         out.writeByte(template_param_length);
-        out.write(template_params);
+        if (template_params != null) {
+            out.write(template_params);
+        }
 
         byte type = getConditionType();
         if (type == CONDITION_TYPE_COLOR_SCALE) {
@@ -432,7 +434,7 @@ protected int getDataSize() {
         len += getFormulaSize(getFormula1());
         len += getFormulaSize(getFormula2());
         len += 2 + getFormulaSize(formula_scale);
-        len += 6 + template_params.length;
+        len += 6 + (template_params == null ? 0 : template_params.length);
 
         byte type = getConditionType();
         if (type == CONDITION_TYPE_COLOR_SCALE) {

diff --git a/poi/src/main/java/org/apache/poi/poifs/crypt/agile/PasswordKeyEncryptor.java b/poi/src/main/java/org/apache/poi/poifs/crypt/agile/PasswordKeyEncryptor.java
@@ -109,7 +109,7 @@ public PasswordKeyEncryptor(Element passwordKey) {
         blockSize = getIntAttr(passwordKey, "blockSize");
         keyBits = getIntAttr(passwordKey, "keyBits");
         hashSize = getIntAttr(passwordKey, "hashSize");
-        cipherAlgorithm = CipherAlgorithm.fromXmlId(passwordKey.getAttribute("cipherAlgorithm"), keyBits);
+        cipherAlgorithm = CipherAlgorithm.fromXmlId(passwordKey.getAttribute("cipherAlgorithm"), keyBits == null ? -1 : keyBits);
         cipherChaining = ChainingMode.fromXmlId(passwordKey.getAttribute("cipherChaining"));
         hashAlgorithm = HashAlgorithm.fromEcmaId(passwordKey.getAttribute("hashAlgorithm"));
         saltValue = getBinAttr(passwordKey, "saltValue");

diff --git a/poi/src/test/java/org/apache/poi/hssf/dev/TestBiffViewer.java b/poi/src/test/java/org/apache/poi/hssf/dev/TestBiffViewer.java
@@ -42,6 +42,7 @@ protected Map<String, Class<? extends Throwable>> getExcludes() {
         excludes.put("61300.xls", IndexOutOfBoundsException.class);
         excludes.put("poi-fuzz.xls", RecordFormatException.class);
         excludes.put("protected_66115.xls", RecordFormatException.class);
+        excludes.put("clusterfuzz-testcase-minimized-POIHSSFFuzzer-5786329142919168.xls", IllegalStateException.class);
 
         return excludes;
     }

diff --git a/poi/src/test/java/org/apache/poi/hssf/dev/TestFormulaViewer.java b/poi/src/test/java/org/apache/poi/hssf/dev/TestFormulaViewer.java
@@ -53,7 +53,7 @@ protected Map<String, Class<? extends Throwable>> getExcludes() {
     @Override
     void runOneFile(File fileIn) throws Exception {
         // replace with System.out for manual tests
-        PrintWriter out = new PrintWriter(new NullWriter());
+        PrintWriter out = new PrintWriter(NullWriter.INSTANCE);
 
         final Function<FormulaRecord, String> lister = (doListFormula) ? this::listFormula : this::parseFormulaRecord;
 

diff --git a/poi/src/test/java/org/apache/poi/hssf/dev/TestRecordLister.java b/poi/src/test/java/org/apache/poi/hssf/dev/TestRecordLister.java
@@ -21,13 +21,15 @@ Licensed to the Apache Software Foundation (ASF) under one or more
 import java.io.InputStream;
 import java.io.PrintWriter;
 import java.util.Locale;
+import java.util.Map;
 
 import org.apache.commons.io.output.NullWriter;
 import org.apache.poi.hssf.record.ContinueRecord;
 import org.apache.poi.hssf.record.Record;
 import org.apache.poi.hssf.record.RecordFactory;
 import org.apache.poi.hssf.record.RecordInputStream;
 import org.apache.poi.poifs.filesystem.POIFSFileSystem;
+import org.apache.poi.util.RecordFormatException;
 
 /**
  * This is a low-level debugging class, which simply prints out what records come in what order.
@@ -40,10 +42,17 @@ Licensed to the Apache Software Foundation (ASF) under one or more
  */
 
 class TestRecordLister extends BaseTestIteratingXLS {
+    @Override
+    protected Map<String, Class<? extends Throwable>> getExcludes() {
+        Map<String, Class<? extends Throwable>> excludes = super.getExcludes();
+        excludes.put("clusterfuzz-testcase-minimized-POIHSSFFuzzer-5786329142919168.xls", RecordFormatException.class);
+        return excludes;
+    }
+
     @Override
     void runOneFile(File fileIn) throws IOException {
         // replace it with System.out if you like it more verbatim
-        PrintWriter out = new PrintWriter(new NullWriter());
+        PrintWriter out = new PrintWriter(NullWriter.INSTANCE);
 
         try (POIFSFileSystem fs = new POIFSFileSystem(fileIn, true);
              InputStream din = BiffViewer.getPOIFSInputStream(fs)) {

diff --git a/poi/src/test/java/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java b/poi/src/test/java/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java
@@ -28,11 +28,13 @@ Licensed to the Apache Software Foundation (ASF) under one or more
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
+import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.OutputStream;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
@@ -69,6 +71,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more
 import org.apache.poi.ss.usermodel.Sheet;
 import org.apache.poi.ss.usermodel.SheetConditionalFormatting;
 import org.apache.poi.ss.usermodel.Workbook;
+import org.apache.poi.ss.usermodel.WorkbookFactory;
 import org.apache.poi.ss.util.CellRangeAddress;
 import org.apache.poi.util.IOUtils;
 import org.apache.poi.util.TempFile;
@@ -1217,4 +1220,15 @@ void checkNewFileForR1C1Refs() throws IOException {
     void createDrawing() {
         // the dimensions for this image are different than for XSSF and SXSSF
     }
+
+    @Test
+    void writeInvalidFile() throws Exception {
+        try (Workbook wb = WorkbookFactory.create(
+                samples.getFile("clusterfuzz-testcase-minimized-POIHSSFFuzzer-5786329142919168.xls"),
+                null, true)) {
+            try (OutputStream out = new ByteArrayOutputStream()) {
+                wb.write(out);
+            }
+        }
+    }
 }
diff --git a/test-data/slideshow/clusterfuzz-testcase-minimized-POIHSLFFuzzer-5231088823566336.ppt b/test-data/slideshow/clusterfuzz-testcase-minimized-POIHSLFFuzzer-5231088823566336.ppt
diff --git a/test-data/spreadsheet/clusterfuzz-testcase-minimized-POIHSSFFuzzer-5786329142919168.xls b/test-data/spreadsheet/clusterfuzz-testcase-minimized-POIHSSFFuzzer-5786329142919168.xls
diff --git a/test-data/spreadsheet/stress.xls b/test-data/spreadsheet/stress.xls