Fix issues found when fuzzing Apache POI via Jazzer

Fix IOUtils.safelyClone to prevent overly large allocations
properly also when length is larger than src.length
Throw an exception instead of an assertion which hides
invalid usage in production builds

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

2 files changed, 56 insertions, 3 deletions

diff --git a/poi/src/main/java/org/apache/poi/util/IOUtils.java b/poi/src/main/java/org/apache/poi/util/IOUtils.java
index 09bb25a9cb..a3ac29ddbb 100644
--- a/poi/src/main/java/org/apache/poi/util/IOUtils.java
+++ b/poi/src/main/java/org/apache/poi/util/IOUtils.java
@@ -501,9 +501,15 @@ public final class IOUtils {
         if (src == null) {
             return null;
         }
-        assert(offset >= 0 && length >= 0 && maxLength >= 0);
-        safelyAllocateCheck(Math.min(src.length-offset,length), maxLength);
-        return Arrays.copyOfRange(src, offset, offset+length);
+
+        if (offset < 0 || length < 0 || maxLength < 0) {
+            throw new RecordFormatException("Invalid offset/length specified: "
+                    + "offset: " + offset + ", lenght: " + length + ", maxLength: " + maxLength);
+        }
+
+        int realLength = Math.min(src.length - offset, length);
+        safelyAllocateCheck(realLength, maxLength);
+        return Arrays.copyOfRange(src, offset, offset+realLength);
     }
 
 
diff --git a/poi/src/test/java/org/apache/poi/util/TestIOUtils.java b/poi/src/test/java/org/apache/poi/util/TestIOUtils.java
index 5a0c5c5bdb..e5a70fa3bb 100644
--- a/poi/src/test/java/org/apache/poi/util/TestIOUtils.java
+++ b/poi/src/test/java/org/apache/poi/util/TestIOUtils.java
@@ -21,6 +21,7 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -442,6 +443,52 @@ final class TestIOUtils {
                 () -> IOUtils.calculateChecksum(new NullInputStream(1, true)));
     }
 
+    @Test
+    void testSafelyCloneNull() {
+        assertNull(IOUtils.safelyClone(null, 0, 0, 0));
+    }
+
+    @Test
+    void testSafelyCloneInvalid() {
+        assertThrows( RecordFormatException.class,
+                () -> IOUtils.safelyClone(new byte[0], -1, 0, 0));
+        assertThrows( RecordFormatException.class,
+                () -> IOUtils.safelyClone(new byte[0], 0, -2, 0));
+        assertThrows( RecordFormatException.class,
+                () -> IOUtils.safelyClone(new byte[0], 0, 0, -3));
+    }
+
+    @Test
+    void testSafelyCloneEmpty() {
+        byte[] bytes = new byte[0];
+        byte[] ret = IOUtils.safelyClone(bytes, 0, 0, 0);
+        assertNotNull(ret);
+        assertEquals(0, ret.length);
+    }
+
+    @Test
+    void testSafelyCloneDataButLengthLimit() {
+        byte[] bytes = new byte[] { 1, 2, 3, 4 };
+        assertThrows( RecordFormatException.class,
+                () -> IOUtils.safelyClone(bytes, 0, bytes.length, 0));
+    }
+
+    @Test
+    void testSafelyCloneData() {
+        byte[] bytes = new byte[] { 1, 2, 3, 4 };
+        byte[] ret = IOUtils.safelyClone(bytes, 0, bytes.length, 100);
+        assertNotNull(ret);
+        assertEquals(4, ret.length);
+    }
+
+    @Test
+    void testSafelyCloneDataHugeLength() {
+        byte[] bytes = new byte[] { 1, 2, 3, 4 };
+        byte[] ret = IOUtils.safelyClone(bytes, 0, Integer.MAX_VALUE, 100);
+        assertNotNull(ret);
+        assertEquals(4, ret.length);
+    }
+
     /**
      * This returns 0 for the first call to skip and then reads
      * as requested.  This tests that the fallback to read() works.