Bug 66425: Avoid NullPointerExceptions and ClassCastExceptions found via poi-fuzz

We try to avoid throwing NullPointerException and ClassCastExceptions, but it was possible
to trigger them

Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=62414
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=62442
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=62450

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1912365 13f79535-47bb-0310-9956-ffa450edef68

10 files changed, 87 insertions, 15 deletions

diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/dev/PPTXMLDump.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/dev/PPTXMLDump.java
index 7940dae77d..e27f5e3b93 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/dev/PPTXMLDump.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/dev/PPTXMLDump.java
@@ -17,6 +17,7 @@
 
 package org.apache.poi.hslf.dev;
 
+import java.io.BufferedWriter;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
@@ -26,7 +27,6 @@ import java.io.Writer;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 
-import org.apache.commons.io.output.StringBuilderWriter;
 import org.apache.commons.io.output.UnsynchronizedByteArrayOutputStream;
 import org.apache.poi.hslf.record.RecordTypes;
 import org.apache.poi.hslf.usermodel.HSLFSlideShow;
@@ -122,6 +122,11 @@ public final class PPTXMLDump {
             int size = (int)LittleEndian.getUInt(data, pos);
             pos += LittleEndianConsts.INT_SIZE;
 
+            if (size < 0) {
+                // stop processing of invalid header data
+                continue;
+            }
+
             //get name of the record by type
             String recname = RecordTypes.forTypeID(type).name();
             write(out, "<"+recname + " info=\""+info+"\" type=\""+type+"\" size=\""+size+"\" offset=\""+(pos-8)+"\"", padding);
@@ -214,12 +219,10 @@ public final class PPTXMLDump {
                     dump.dump(out);
                     out.close();
                 } else {
-                    StringBuilderWriter out = new StringBuilderWriter(1024);
-                    dump.dump(out);
-                    System.out.println(out);
+                    dump.dump(new BufferedWriter(
+                            new OutputStreamWriter(System.out, StandardCharsets.UTF_8)));
                 }
             }
-
         }
     }
 
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/dev/SlideShowDumper.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/dev/SlideShowDumper.java
index d457ee63b7..c3c6e4a146 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/dev/SlideShowDumper.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/dev/SlideShowDumper.java
@@ -194,6 +194,11 @@ public final class SlideShowDumper {
             pos += 8;
             out.printf(Locale.ROOT, ind + "That's a %2$s%n", "", recordName);
 
+            if (len < 0 /*|| len > Integer.MAX_VALUE*/) {
+                // stop processing of invalid header data
+                continue;
+            }
+
             // Now check if it's a container or not
             int container = opt & 0x0f;
 
@@ -219,7 +224,7 @@ public final class SlideShowDumper {
                 }
             }
 
-            pos += (int) len;
+            pos += (int) Math.min(len, Integer.MAX_VALUE);
         }
     }
 
diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/DocumentEncryptionAtom.java b/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/DocumentEncryptionAtom.java
index f2f05087da..64f1b83646 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/DocumentEncryptionAtom.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hslf/record/DocumentEncryptionAtom.java
@@ -26,8 +26,10 @@ import java.util.function.Supplier;
 
 import org.apache.poi.EncryptedDocumentException;
 import org.apache.poi.poifs.crypt.CipherAlgorithm;
+import org.apache.poi.poifs.crypt.EncryptionHeader;
 import org.apache.poi.poifs.crypt.EncryptionInfo;
 import org.apache.poi.poifs.crypt.EncryptionMode;
+import org.apache.poi.poifs.crypt.EncryptionVerifier;
 import org.apache.poi.poifs.crypt.HashAlgorithm;
 import org.apache.poi.poifs.crypt.cryptoapi.CryptoAPIEncryptionHeader;
 import org.apache.poi.poifs.crypt.cryptoapi.CryptoAPIEncryptionVerifier;
@@ -118,8 +120,16 @@ public final class DocumentEncryptionAtom extends PositionDependentRecordAtom {
         bos.writeShort(ei.getVersionMinor());
         bos.writeInt(ei.getEncryptionFlags());
 
-        ((CryptoAPIEncryptionHeader)ei.getHeader()).write(bos);
-        ((CryptoAPIEncryptionVerifier)ei.getVerifier()).write(bos);
+        final EncryptionHeader header = ei.getHeader();
+        if (!(header instanceof CryptoAPIEncryptionHeader)) {
+            throw new IllegalStateException("Had unexpected type of header: " + header.getClass());
+        }
+        ((CryptoAPIEncryptionHeader) header).write(bos);
+        final EncryptionVerifier verifier = ei.getVerifier();
+        if (!(verifier instanceof CryptoAPIEncryptionVerifier)) {
+            throw new IllegalStateException("Had unexpected type of verifier: " + verifier.getClass());
+        }
+        ((CryptoAPIEncryptionVerifier) verifier).write(bos);
 
         // Header
         LittleEndian.putInt(_header, 4, bos.getWriteIndex());
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 61f190a9c5..7848f688cd 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
@@ -64,6 +64,7 @@ public abstract class BaseTestPPTIterating {
         EXCLUDED.put("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6416153805979648.ppt", Exception.class);
         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);
     }
 
     public static Stream<Arguments> files() {
diff --git a/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestPPDrawingTextListing.java b/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestPPDrawingTextListing.java
index 206db2b20a..fc39881195 100644
--- a/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestPPDrawingTextListing.java
+++ b/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestPPDrawingTextListing.java
@@ -19,12 +19,20 @@ package org.apache.poi.hslf.dev;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.util.HashSet;
+import java.util.Set;
 
 import org.apache.poi.EmptyFileException;
 import org.junit.jupiter.api.Test;
 
 public class TestPPDrawingTextListing extends BaseTestPPTIterating {
+    static final Set<String> LOCAL_EXCLUDED = new HashSet<>();
+    static {
+        LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIFuzzer-5681320547975168.ppt");
+    }
+
     @Test
     void testMain() throws IOException {
         // calls System.exit(): PPDrawingTextListing.main(new String[0]);
@@ -33,6 +41,17 @@ public class TestPPDrawingTextListing extends BaseTestPPTIterating {
 
     @Override
     void runOneFile(File pFile) throws Exception {
-        PPDrawingTextListing.main(new String[]{pFile.getAbsolutePath()});
+        try {
+            PPDrawingTextListing.main(new String[]{pFile.getAbsolutePath()});
+        } catch (IndexOutOfBoundsException | IOException e) {
+            if (!LOCAL_EXCLUDED.contains(pFile.getName())) {
+                throw e;
+            }
+        }
+
+        // work around one file which works here but not in other tests
+        if (pFile.getName().equals("clusterfuzz-testcase-minimized-POIFuzzer-5681320547975168.ppt")) {
+            throw new FileNotFoundException();
+        }
     }
 }
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 90965f727d..0f7b38a6b8 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
@@ -34,6 +34,8 @@ public class TestPPTXMLDump extends BaseTestPPTIterating {
     static {
         LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-5306877435838464.ppt");
         LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6032591399288832.ppt");
+        LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6360479850954752.ppt");
+        LOCAL_EXCLUDED.add("ppt_with_png_encrypted.ppt");
     }
 
     @Test
@@ -52,14 +54,18 @@ public class TestPPTXMLDump extends BaseTestPPTIterating {
     void runOneFile(File pFile) throws Exception {
         try {
            PPTXMLDump.main(new String[]{pFile.getAbsolutePath()});
+            if (LOCAL_EXCLUDED.contains(pFile.getName())) {
+                throw new IllegalStateException("Expected failure for file " + pFile + ", but processing did not throw an exception");
+            }
         } catch (IndexOutOfBoundsException | IOException e) {
             if (!LOCAL_EXCLUDED.contains(pFile.getName())) {
                 throw e;
             }
         }
 
-        // work around one file which works here but not in other tests
-        if (pFile.getName().equals("clusterfuzz-testcase-minimized-POIFuzzer-5429732352851968.ppt")) {
+        // 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")) {
             throw new FileNotFoundException();
         }
     }
diff --git a/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideIdListing.java b/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideIdListing.java
index f3afc851fb..f86247c86b 100644
--- a/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideIdListing.java
+++ b/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideIdListing.java
@@ -31,6 +31,7 @@ public class TestSlideIdListing extends BaseTestPPTIterating {
     static final Set<String> LOCAL_EXCLUDED = new HashSet<>();
     static {
         LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-5306877435838464.ppt");
+        LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6360479850954752.ppt");
     }
 
     @Test
@@ -46,7 +47,7 @@ public class TestSlideIdListing extends BaseTestPPTIterating {
     void runOneFile(File pFile) throws Exception {
         try {
             SlideIdListing.main(new String[]{pFile.getAbsolutePath()});
-        } catch (IllegalArgumentException e) {
+        } catch (RuntimeException e) {
             if (!LOCAL_EXCLUDED.contains(pFile.getName())) {
                 throw e;
             }
diff --git a/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideShowDumper.java b/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideShowDumper.java
index 1b62833aec..06a742c191 100644
--- a/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideShowDumper.java
+++ b/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideShowDumper.java
@@ -38,6 +38,7 @@ public class TestSlideShowDumper extends BaseTestPPTIterating {
         FAILING.add("cryptoapi-proc2356.ppt");
         FAILING.add("41384.ppt");
         FAILING.add("bug56240.ppt");
+        FAILING.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6360479850954752.ppt");
     }
 
     @Test
@@ -66,7 +67,7 @@ public class TestSlideShowDumper extends BaseTestPPTIterating {
             }
         } catch (FileNotFoundException e) {
             // some old files are not detected correctly
-            if(!OLD_FILES.contains(pFile.getName())) {
+            if(!FAILING.contains(pFile.getName()) && !OLD_FILES.contains(pFile.getName())) {
                 throw e;
             }
         }
diff --git a/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideShowRecordDumper.java b/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideShowRecordDumper.java
index 4392ec603a..2a45921a11 100644
--- a/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideShowRecordDumper.java
+++ b/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestSlideShowRecordDumper.java
@@ -20,12 +20,19 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import java.io.File;
 import java.io.IOException;
+import java.util.HashSet;
+import java.util.Set;
 
 import org.apache.poi.EmptyFileException;
 import org.apache.poi.hslf.HSLFTestDataSamples;
 import org.junit.jupiter.api.Test;
 
 public class TestSlideShowRecordDumper extends BaseTestPPTIterating {
+    static final Set<String> LOCAL_EXCLUDED = new HashSet<>();
+    static {
+        LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6360479850954752.ppt");
+    }
+
     @Test
     void testMain() throws IOException {
         SlideShowRecordDumper.main(new String[] {
@@ -47,6 +54,12 @@ public class TestSlideShowRecordDumper extends BaseTestPPTIterating {
 
     @Override
     void runOneFile(File pFile) throws Exception {
-        SlideShowRecordDumper.main(new String[]{pFile.getAbsolutePath()});
+        try {
+            SlideShowRecordDumper.main(new String[]{pFile.getAbsolutePath()});
+        } catch (IllegalStateException e) {
+            if (!LOCAL_EXCLUDED.contains(pFile.getName())) {
+                throw e;
+            }
+        }
     }
 }
\ No newline at end of file
diff --git a/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestUserEditAndPersistListing.java b/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestUserEditAndPersistListing.java
index c284ce1ba3..fab8679776 100644
--- a/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestUserEditAndPersistListing.java
+++ b/poi-scratchpad/src/test/java/org/apache/poi/hslf/dev/TestUserEditAndPersistListing.java
@@ -20,11 +20,18 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import java.io.File;
 import java.io.IOException;
+import java.util.HashSet;
+import java.util.Set;
 
 import org.apache.poi.EmptyFileException;
 import org.junit.jupiter.api.Test;
 
 public class TestUserEditAndPersistListing extends BaseTestPPTIterating {
+    static final Set<String> LOCAL_EXCLUDED = new HashSet<>();
+    static {
+        LOCAL_EXCLUDED.add("clusterfuzz-testcase-minimized-POIHSLFFuzzer-6360479850954752.ppt");
+    }
+
     @Test
     void testMain() throws IOException {
         // calls System.exit(): UserEditAndPersistListing.main(new String[0]);
@@ -33,6 +40,12 @@ public class TestUserEditAndPersistListing extends BaseTestPPTIterating {
 
     @Override
     void runOneFile(File pFile) throws Exception {
-        UserEditAndPersistListing.main(new String[]{pFile.getAbsolutePath()});
+        try {
+            UserEditAndPersistListing.main(new String[]{pFile.getAbsolutePath()});
+        } catch (IllegalStateException e) {
+            if (!LOCAL_EXCLUDED.contains(pFile.getName())) {
+                throw e;
+            }
+        }
     }
 }
\ No newline at end of file