From: Dominik Stadler Date: Wed, 20 Sep 2023 14:55:19 +0000 (+0000) Subject: Bug 66425: Avoid exceptions found via poi-fuzz X-Git-Tag: REL_5_2_4~5 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=ce919673c4e935e8c756c91938f973d5c9a23ddb;p=poi.git 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 --- 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 index 2c061e0960..915abe819a 100644 --- 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 @@ import org.w3c.dom.Element; 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 index ef146dda23..7cc9272781 100644 --- 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 @@ import java.util.stream.Stream; 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 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 index 0f7b38a6b8..ca4eb18ac5 100644 --- 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 @@ public class TestPPTXMLDump extends BaseTestPPTIterating { // 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 index 4381c52a18..2b0fb59c5e 100644 --- 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 final class CFRule12Record extends CFRuleBase implements FutureRecord { 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 @@ public final class CFRule12Record extends CFRuleBase implements FutureRecord { 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 index c31caaba40..5d8c0cb952 100644 --- 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 class PasswordKeyEncryptor { 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 index 107c87668b..3b8ee50d96 100644 --- 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 @@ class TestBiffViewer extends BaseTestIteratingXLS { 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 index 3b10498e69..29b31dec81 100644 --- 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 @@ class TestFormulaViewer extends BaseTestIteratingXLS { @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 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 index dbcb3c0618..9135eab884 100644 --- 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,6 +21,7 @@ import java.io.IOException; 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; @@ -28,6 +29,7 @@ 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 @@ import org.apache.poi.poifs.filesystem.POIFSFileSystem; */ class TestRecordLister extends BaseTestIteratingXLS { + @Override + protected Map> getExcludes() { + Map> 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 index 6b9a1575f6..eb87441cd3 100644 --- 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 @@ import static org.junit.jupiter.api.Assertions.assertThrows; 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 @@ import org.apache.poi.ss.usermodel.Row; 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 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook { 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 new file mode 100644 index 0000000000..dbf00b3265 Binary files /dev/null and b/test-data/slideshow/clusterfuzz-testcase-minimized-POIHSLFFuzzer-5231088823566336.ppt differ diff --git a/test-data/spreadsheet/clusterfuzz-testcase-minimized-POIHSSFFuzzer-5786329142919168.xls b/test-data/spreadsheet/clusterfuzz-testcase-minimized-POIHSSFFuzzer-5786329142919168.xls new file mode 100644 index 0000000000..c83bbf76e2 Binary files /dev/null and b/test-data/spreadsheet/clusterfuzz-testcase-minimized-POIHSSFFuzzer-5786329142919168.xls differ diff --git a/test-data/spreadsheet/stress.xls b/test-data/spreadsheet/stress.xls index 8d0313eee0..46a4282e1e 100644 Binary files a/test-data/spreadsheet/stress.xls and b/test-data/spreadsheet/stress.xls differ