diff --git a/poi-scratchpad/src/main/java/org/apache/poi/hdgf/pointers/PointerFactory.java b/poi-scratchpad/src/main/java/org/apache/poi/hdgf/pointers/PointerFactory.java index 35c475b175e..17fc3ebe5ea 100644 --- a/poi-scratchpad/src/main/java/org/apache/poi/hdgf/pointers/PointerFactory.java +++ b/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,8 +65,12 @@ 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 { @@ -64,6 +78,13 @@ public Pointer createPointer(byte[] data, int offset) { } } + 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 diff --git a/poi-scratchpad/src/test/java/org/apache/poi/hdgf/pointers/TestPointerFactory.java b/poi-scratchpad/src/test/java/org/apache/poi/hdgf/pointers/TestPointerFactory.java index 641f8909b36..b9a52044a88 100644 --- a/poi-scratchpad/src/test/java/org/apache/poi/hdgf/pointers/TestPointerFactory.java +++ b/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); diff --git a/test-data/poi-integration-exceptions.csv b/test-data/poi-integration-exceptions.csv index 31e40f8f922..c3f28066973 100644 --- a/test-data/poi-integration-exceptions.csv +++ b/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: but was: , -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",