Guard HDGF PointerFactory offset/length uint32 narrowing

poi-scratchpad/src/main/java/org/apache/poi/hdgf/pointers/PointerFactory.java

@@ -20,6 +20,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more
 import org.apache.poi.hdgf.streams.PointerContainingStream;
 import org.apache.poi.util.IOUtils;
 import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.RecordFormatException;
 
 /**
  * Factor class to create the appropriate pointers, based on the version
@@ -45,8 +46,17 @@ public Pointer createPointer(byte[] data, int offset) {
             p = new PointerV6();
             p.setType(LittleEndian.getInt(data, offset));
             p.setAddress((int)LittleEndian.getUInt(data, offset+4));
-            p.setOffset((int)LittleEndian.getUInt(data, offset+8));
-            p.setLength((int)LittleEndian.getUInt(data, offset+12));
+            // Offset and Length flow into Stream.createStream as the (offset, length)
+            // pair handed to StreamStore / CompressedStreamStore -> IOUtils.safelyClone.
+            // Validate the uint32 values up-front: throw RecordFormatException with the
+            // offending value rather than letting a wrapped int reach the downstream
+            // bounds check (matches the EMF header description fix in PR #1060).
+            final long v6Offset = LittleEndian.getUInt(data, offset+8);
+            final long v6Length = LittleEndian.getUInt(data, offset+12);
+            checkPointerOffset(v6Offset);
+            IOUtils.safelyAllocateCheck(v6Length, Integer.MAX_VALUE);
+            p.setOffset((int)v6Offset);
+            p.setLength((int)v6Length);
             p.setFormat(LittleEndian.getShort(data, offset+16));
 
             return p;
@@ -55,15 +65,26 @@ public Pointer createPointer(byte[] data, int offset) {
             p.setType(LittleEndian.getShort(data, offset));
             p.setFormat(LittleEndian.getShort(data, offset+2));
             p.setAddress((int)LittleEndian.getUInt(data, offset+4));
-            p.setOffset((int)LittleEndian.getUInt(data, offset+8));
-            p.setLength((int)LittleEndian.getUInt(data, offset+12));
+            final long v5Offset = LittleEndian.getUInt(data, offset+8);
+            final long v5Length = LittleEndian.getUInt(data, offset+12);
+            checkPointerOffset(v5Offset);
+            IOUtils.safelyAllocateCheck(v5Length, Integer.MAX_VALUE);
+            p.setOffset((int)v5Offset);
+            p.setLength((int)v5Length);
 
             return p;
         } else {
             throw new OldVisioFormatException("Visio files with versions below 5 are not supported, yours was " + version);
         }
     }
 
+    private static void checkPointerOffset(long unsignedOffset) {
+        if (unsignedOffset > Integer.MAX_VALUE) {
+            throw new RecordFormatException(
+                    "HDGF Pointer offset " + unsignedOffset + " exceeds Integer.MAX_VALUE");
+        }
+    }
+
     /**
      * Parsers the {@link PointerContainingStream} contents and
      *  creates all the child Pointers for it

poi-scratchpad/src/test/java/org/apache/poi/hdgf/pointers/TestPointerFactory.java

@@ -22,6 +22,8 @@ Licensed to the Apache Software Foundation (ASF) under one or more
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.RecordFormatException;
 import org.junit.jupiter.api.Test;
 
 /**
@@ -134,6 +136,96 @@ void testCreateV6() {
         assertFalse(d.destinationHasPointers());
     }
 
+    /**
+     * A v6+ Pointer reads its Offset and Length fields as 32-bit unsigned
+     * integers, then narrows them to int and hands the pair to
+     * {@code Stream.createStream} -> {@code StreamStore} /
+     * {@code CompressedStreamStore} -> {@code IOUtils.safelyClone}. A
+     * crafted file with Length > Integer.MAX_VALUE used to be silently
+     * narrowed via a plain {@code (int)} cast, letting a wrapped value flow
+     * into the downstream bounds check. Validate the uint32 values up-front
+     * via {@code IOUtils.safelyAllocateCheck} (length) and an explicit
+     * {@code RecordFormatException} (offset) so the failure carries the
+     * actual offending value, not a bare "integer overflow".
+     */
+    @Test
+    void testCreateV6RejectsOversizedLength() {
+        PointerFactory pf = new PointerFactory(11);
+
+        byte[] ptr = new byte[18];
+        LittleEndian.putInt  (ptr, 0,  0x16);              // type
+        LittleEndian.putUInt (ptr, 4,  0x0143aff4L);       // address
+        LittleEndian.putUInt (ptr, 8,  0x80L);             // offset (valid)
+        LittleEndian.putUInt (ptr, 12, 0x80000001L);       // length: would wrap to negative int
+        LittleEndian.putShort(ptr, 16, (short)0x46);       // format
+
+        assertThrows(RecordFormatException.class, () -> pf.createPointer(ptr, 0));
+    }
+
+    @Test
+    void testCreateV6RejectsOversizedOffset() {
+        PointerFactory pf = new PointerFactory(11);
+
+        byte[] ptr = new byte[18];
+        LittleEndian.putInt  (ptr, 0,  0x16);
+        LittleEndian.putUInt (ptr, 4,  0x0143aff4L);
+        LittleEndian.putUInt (ptr, 8,  0xFFFFFFFFL);       // offset: would wrap to -1
+        LittleEndian.putUInt (ptr, 12, 0x54L);
+        LittleEndian.putShort(ptr, 16, (short)0x46);
+
+        assertThrows(RecordFormatException.class, () -> pf.createPointer(ptr, 0));
+    }
+
+    @Test
+    void testCreateV5RejectsOversizedLength() {
+        PointerFactory pf = new PointerFactory(5);
+
+        byte[] ptr = new byte[16];
+        LittleEndian.putShort(ptr, 0,  (short)0x14);
+        LittleEndian.putShort(ptr, 2,  (short)0x52);
+        LittleEndian.putUInt (ptr, 4,  0x011eb2acL);
+        LittleEndian.putUInt (ptr, 8,  0x1dd4L);
+        LittleEndian.putUInt (ptr, 12, 0x80000001L);       // length: would wrap to negative
+
+        assertThrows(RecordFormatException.class, () -> pf.createPointer(ptr, 0));
+    }
+
+    @Test
+    void testCreateV5RejectsOversizedOffset() {
+        PointerFactory pf = new PointerFactory(5);
+
+        byte[] ptr = new byte[16];
+        LittleEndian.putShort(ptr, 0,  (short)0x14);
+        LittleEndian.putShort(ptr, 2,  (short)0x52);
+        LittleEndian.putUInt (ptr, 4,  0x011eb2acL);
+        LittleEndian.putUInt (ptr, 8,  0xFFFFFFFFL);       // offset: would wrap to -1
+        LittleEndian.putUInt (ptr, 12, 0x14dL);
+
+        assertThrows(RecordFormatException.class, () -> pf.createPointer(ptr, 0));
+    }
+
+    /**
+     * Values up to {@code Integer.MAX_VALUE} (still nonsensically large but
+     * representable) must continue to parse — the hardening is only meant to
+     * catch the silent-narrowing case, not to introduce a new lower ceiling.
+     * Downstream {@code IOUtils.safelyClone} handles real bounding.
+     */
+    @Test
+    void testCreateV6AcceptsMaxIntOffsetAndLength() {
+        PointerFactory pf = new PointerFactory(11);
+
+        byte[] ptr = new byte[18];
+        LittleEndian.putInt  (ptr, 0,  0x16);
+        LittleEndian.putUInt (ptr, 4,  0x0143aff4L);
+        LittleEndian.putUInt (ptr, 8,  Integer.MAX_VALUE & 0xFFFFFFFFL);
+        LittleEndian.putUInt (ptr, 12, Integer.MAX_VALUE & 0xFFFFFFFFL);
+        LittleEndian.putShort(ptr, 16, (short)0x46);
+
+        Pointer p = pf.createPointer(ptr, 0);
+        assertEquals(Integer.MAX_VALUE, p.getOffset());
+        assertEquals(Integer.MAX_VALUE, p.getLength());
+    }
+
     @Test
     void testCreateV6FromMid() {
         PointerFactory pf = new PointerFactory(11);

test-data/poi-integration-exceptions.csv

@@ -185,10 +185,9 @@ slideshow/clusterfuzz-testcase-minimized-POIHSLFFuzzer-6710128412590080.ppt,"han
 slideshow/clusterfuzz-testcase-minimized-POIHSLFFuzzer-6710128412590080.ppt,extract,HPSF,,java.lang.IllegalArgumentException,Had unexpected type of entry for name,
 slideshow/clusterfuzz-testcase-minimized-POIHSLFFuzzer-6710128412590080.ppt,additional,HPSF,,java.lang.IllegalStateException,Buffer overrun,
 slideshow/clusterfuzz-testcase-minimized-POIHSLFFuzzer-6710128412590080.ppt,handle,HPSF,,org.opentest4j.AssertionFailedError,expected: <true> but was: <false>,
-diagram/clusterfuzz-testcase-minimized-POIHDGFFuzzer-5947849161179136.vsd,extract,"HDGF,HPSF",,java.lang.IllegalArgumentException,"Encountered too deep nesting, cannot process stream",
-diagram/clusterfuzz-testcase-minimized-POIHDGFFuzzer-5947849161179136.vsd,handle,HDGF,,java.lang.IllegalArgumentException,"Encountered too deep nesting, cannot process stream",
+diagram/clusterfuzz-testcase-minimized-POIHDGFFuzzer-5947849161179136.vsd,extract,"HDGF,HPSF",,org.apache.poi.util.RecordFormatException,Can't allocate an array > 2147483647,
+diagram/clusterfuzz-testcase-minimized-POIHDGFFuzzer-5947849161179136.vsd,handle,HDGF,,org.apache.poi.util.RecordFormatException,Can't allocate an array > 2147483647,
 diagram/clusterfuzz-testcase-minimized-POIHDGFFuzzer-5947849161179136.vsd,"handle,additional",HPSF,,java.util.NoSuchElementException,Can't read past the end of the stream,
-diagram/clusterfuzz-testcase-minimized-POIHDGFFuzzer-5947849161179136.vsd,handle,HDGF,,org.apache.poi.util.RecordFormatException,Can't allocate an array of length < 0,
 hsmf/clusterfuzz-testcase-minimized-POIHSMFFuzzer-4735011465854976.msg,extract,"HSMF,HPSF",,java.lang.IllegalArgumentException,ChunkId and type of chunk did not match,
 hsmf/clusterfuzz-testcase-minimized-POIHSMFFuzzer-4735011465854976.msg,handle,HSMF,,java.lang.IllegalArgumentException,ChunkId and type of chunk did not match,
 diagram/clusterfuzz-testcase-minimized-POIHDGFFuzzer-4913778037489664.vsd,extract,"HDGF,HPSF",,java.lang.IllegalArgumentException,"Encountered too deep nesting, cannot process stream",