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

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);
     }
 
 

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.