This is an automated email from the ASF dual-hosted git repository.
pjfanning pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/poi.git
The following commit(s) were added to refs/heads/trunk by this push:
new e81982e6c3 Validate HDGF v6+ chunk Length before narrowing to int
(#1075)
e81982e6c3 is described below
commit e81982e6c3f6e861a84a183faf53c7daef2b5e63
Author: metsw24-max <[email protected]>
AuthorDate: Thu May 14 19:42:09 2026 +0530
Validate HDGF v6+ chunk Length before narrowing to int (#1075)
---
.../org/apache/poi/hdgf/chunks/ChunkHeader.java | 6 ++-
.../org/apache/poi/hdgf/chunks/TestChunks.java | 46 ++++++++++++++++++++++
2 files changed, 51 insertions(+), 1 deletion(-)
diff --git
a/poi-scratchpad/src/main/java/org/apache/poi/hdgf/chunks/ChunkHeader.java
b/poi-scratchpad/src/main/java/org/apache/poi/hdgf/chunks/ChunkHeader.java
index 8f31139f53..209550a786 100644
--- a/poi-scratchpad/src/main/java/org/apache/poi/hdgf/chunks/ChunkHeader.java
+++ b/poi-scratchpad/src/main/java/org/apache/poi/hdgf/chunks/ChunkHeader.java
@@ -50,7 +50,11 @@ public abstract class ChunkHeader {
ch.setType((int) LittleEndian.getUInt(data, offset));
ch.setId((int) LittleEndian.getUInt(data, offset + 4));
ch.setUnknown1((int) LittleEndian.getUInt(data, offset + 8));
- ch.setLength((int) LittleEndian.getUInt(data, offset + 12));
+ // Match the v4/v5 branch below: reject lengths that would silently
+ // truncate when narrowing the uint32 to a signed int, rather than
+ // letting a wrapped value flow into the offset arithmetic in
+ // ChunkFactory.createChunk (offset + getLength() + sizeInBytes).
+ ch.setLength(Math.toIntExact(LittleEndian.getUInt(data, offset +
12)));
ch.setUnknown2(LittleEndian.getShort(data, offset + 16));
ch.setUnknown3(LittleEndian.getUByte(data, offset + 18));
diff --git
a/poi-scratchpad/src/test/java/org/apache/poi/hdgf/chunks/TestChunks.java
b/poi-scratchpad/src/test/java/org/apache/poi/hdgf/chunks/TestChunks.java
index 58254a5933..a5a74a4853 100644
--- a/poi-scratchpad/src/test/java/org/apache/poi/hdgf/chunks/TestChunks.java
+++ b/poi-scratchpad/src/test/java/org/apache/poi/hdgf/chunks/TestChunks.java
@@ -20,12 +20,14 @@ package org.apache.poi.hdgf.chunks;
import static org.junit.jupiter.api.Assertions.assertEquals;
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;
import java.io.IOException;
import org.apache.poi.hdgf.chunks.ChunkFactory.CommandDefinition;
import org.apache.poi.poifs.storage.RawDataUtil;
+import org.apache.poi.util.LittleEndian;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
@@ -62,6 +64,50 @@ public final class TestChunks {
assertTrue(header.hasSeparator());
}
+ /**
+ * A v6+ chunk header reads its Length field as a 32-bit unsigned integer.
+ * A crafted file with Length > Integer.MAX_VALUE used to be silently
+ * narrowed via a plain {@code (int)} cast, letting the wrapped value flow
+ * into the offset arithmetic in {@code ChunkFactory.createChunk}
+ * ({@code offset + getLength() + sizeInBytes}). Match the v4/v5 branch,
+ * which has always used {@code Math.toIntExact} for its Length field, and
+ * reject the input up-front.
+ */
+ @Test
+ void testV11RejectsOversizedLength() {
+ // 19-byte v6+ header: type, id, unknown1, length, unknown2 (short),
unknown3 (ubyte)
+ byte[] header = new byte[19];
+ LittleEndian.putUInt(header, 0, 0x46L); // type
+ LittleEndian.putUInt(header, 4, 0xFFFFFFFFL); // id (allowed -1
sentinel)
+ LittleEndian.putUInt(header, 8, 0x02L); // unknown1
+ LittleEndian.putUInt(header, 12, 0x80000001L); // length: would
wrap to a negative int
+ LittleEndian.putShort(header, 16, (short)0);
+ header[18] = 0;
+
+ assertThrows(ArithmeticException.class,
+ () -> ChunkHeader.createChunkHeader(11, header, 0));
+ }
+
+ /**
+ * Lengths 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 checks (e.g. {@code IOUtils.safelyClone}) handle bounding.
+ */
+ @Test
+ void testV11AcceptsMaxIntLength() {
+ byte[] header = new byte[19];
+ LittleEndian.putUInt(header, 0, 0x46L);
+ LittleEndian.putUInt(header, 4, 0x01L);
+ LittleEndian.putUInt(header, 8, 0x02L);
+ LittleEndian.putUInt(header, 12, Integer.MAX_VALUE & 0xFFFFFFFFL);
+ LittleEndian.putShort(header, 16, (short)0);
+ header[18] = 0;
+
+ ChunkHeader h = ChunkHeader.createChunkHeader(11, header, 0);
+ assertEquals(Integer.MAX_VALUE, h.getLength());
+ }
+
@Test
void testChunkHeaderB() {
ChunkHeader h = ChunkHeader.createChunkHeader(11, data_b, 0);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
