This is an automated email from the ASF dual-hosted git repository.
tballison pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tika.git
The following commit(s) were added to refs/heads/main by this push:
new 6034506703 TIKA-4783 -- improve chm parsing (#2942)
6034506703 is described below
commit 6034506703c31b5c2746f41ccd96af14cd1d7e2d
Author: Tim Allison <[email protected]>
AuthorDate: Mon Jul 13 10:08:31 2026 -0400
TIKA-4783 -- improve chm parsing (#2942)
---
.../tika/parser/microsoft/chm/ChmCommons.java | 13 +-
.../microsoft/chm/ChmDirectoryListingSet.java | 55 +++---
.../tika/parser/microsoft/chm/ChmExtractor.java | 3 +
.../tika/parser/microsoft/chm/ChmItsfHeader.java | 3 +-
.../tika/parser/microsoft/chm/ChmItspHeader.java | 3 +-
.../tika/parser/microsoft/chm/ChmLzxBlock.java | 8 +-
.../parser/microsoft/chm/ChmLzxcControlData.java | 3 +-
.../parser/microsoft/chm/ChmLzxcResetTable.java | 14 +-
.../tika/parser/microsoft/chm/ChmPmgiHeader.java | 3 +-
.../tika/parser/microsoft/chm/ChmPmglHeader.java | 3 +-
.../tika/parser/microsoft/chm/ChmSection.java | 15 +-
.../microsoft/chm/TestChmLzxcControlData.java | 18 ++
.../parser/microsoft/chm/TestChmMalformed.java | 194 +++++++++++++++++++++
.../tika/parser/microsoft/chm/TestParameters.java | 3 +-
14 files changed, 289 insertions(+), 49 deletions(-)
diff --git
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmCommons.java
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmCommons.java
index 102a3691c2..25915c97b7 100644
---
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmCommons.java
+++
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmCommons.java
@@ -307,7 +307,7 @@ public class ChmCommons {
checkCopyOfRangeParams(original, from, to);
int newLength = to - from;
if (newLength < 0) {
- throw new IllegalArgumentException(from + " > " + to);
+ throw new ChmParsingException(from + " > " + to);
}
byte[] copy = new byte[newLength];
@@ -315,18 +315,19 @@ public class ChmCommons {
return copy;
}
- private static void checkCopyOfRangeParams(byte[] original, int from, int
to) {
+ private static void checkCopyOfRangeParams(byte[] original, int from, int
to)
+ throws ChmParsingException {
if (original == null) {
- throw new NullPointerException("array is null");
+ throw new ChmParsingException("array is null");
}
if (from < 0) {
- throw new IllegalArgumentException(from + " should be > 0");
+ throw new ChmParsingException(from + " should be > 0");
}
if (to < 0) {
- throw new IllegalArgumentException(to + " should be > 0");
+ throw new ChmParsingException(to + " should be > 0");
}
if (to > original.length) {
- throw new IllegalArgumentException("can't copy beyond array
length");
+ throw new ChmParsingException("can't copy beyond array length");
}
}
diff --git
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmDirectoryListingSet.java
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmDirectoryListingSet.java
index ddca6a9aef..e852f3a8f8 100644
---
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmDirectoryListingSet.java
+++
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmDirectoryListingSet.java
@@ -18,7 +18,6 @@ package org.apache.tika.parser.microsoft.chm;
import static java.nio.charset.StandardCharsets.UTF_8;
-import java.math.BigInteger;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
@@ -220,17 +219,12 @@ public class ChmDirectoryListingSet {
//setPlaceHolder(header_len);
while (placeHolder > 0 && placeHolder < dir_chunk.length -
PMGLheader.getFreeSpace()
/*&& dir_chunk[placeHolder - 1] != 115*/) {
- //get entry name length
- int strlen = 0;// = getEncint(data);
- byte temp;
- while ((temp = dir_chunk[placeHolder++]) >= 0x80) {
- strlen <<= 7;
- strlen += temp & 0x7f;
- }
-
- strlen = (strlen << 7) + temp & 0x7f;
+ //get entry name length (variable-length ENCINT). The old
inline
+ //decoder compared a signed byte to 0x80 (never true), so it
read a
+ //single byte and capped the length at 127, corrupting longer
names.
+ int strlen = getEncint(dir_chunk);
- if (strlen > dir_chunk.length) {
+ if (strlen < 0 || strlen > dir_chunk.length) {
throw new ChmParsingException("Bad data of a string
length.");
}
@@ -324,22 +318,33 @@ public class ChmDirectoryListingSet {
* @param data_chunk
* @return
*/
- private int getEncint(byte[] data_chunk) {
- byte ob;
- BigInteger bi = BigInteger.ZERO;
- byte[] nb = new byte[1];
-
- if (placeHolder < data_chunk.length) {
- while ((ob = data_chunk[placeHolder]) < 0) {
- nb[0] = (byte) ((ob & 0x7f));
- bi = bi.shiftLeft(7).add(new BigInteger(nb));
- setPlaceHolder(placeHolder + 1);
- }
- nb[0] = (byte) ((ob & 0x7f));
- bi = bi.shiftLeft(7).add(new BigInteger(nb));
+ private int getEncint(byte[] dataChunk) {
+ int start = placeHolder;
+ //advance past the continuation bytes (high bit set) and the
terminating byte
+ while (placeHolder < dataChunk.length && dataChunk[placeHolder] < 0) {
+ setPlaceHolder(placeHolder + 1);
+ }
+ if (placeHolder < dataChunk.length) {
setPlaceHolder(placeHolder + 1);
}
- return bi.intValue();
+ return (int) decodeEncint(dataChunk, start);
+ }
+
+ /**
+ * Decodes a variable-length ENCINT beginning at {@code offset}: each byte
+ * contributes its low 7 bits (most-significant group first) and the high
bit
+ * signals that another byte follows. Package-private for testing.
+ */
+ static long decodeEncint(byte[] data, int offset) {
+ long value = 0;
+ while (offset < data.length && data[offset] < 0) {
+ value = (value << 7) | (data[offset] & 0x7f);
+ offset++;
+ }
+ if (offset < data.length) {
+ value = (value << 7) | (data[offset] & 0x7f);
+ }
+ return value;
}
/**
diff --git
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmExtractor.java
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmExtractor.java
index f63e0f30f2..c40b5687cf 100644
---
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmExtractor.java
+++
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmExtractor.java
@@ -131,7 +131,10 @@ public class ChmExtractor {
setLzxBlocksCache(new ArrayList<>());
} catch (IOException e) {
+ //don't swallow: propagate so a broken stream surfaces instead of
leaving
+ //a half-built extractor that NPEs later in ChmParser
LOG.warn("IOException parsing chm file", e);
+ throw e;
}
}
diff --git
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmItsfHeader.java
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmItsfHeader.java
index 59d5458424..9e195d81d3 100644
---
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmItsfHeader.java
+++
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmItsfHeader.java
@@ -395,10 +395,11 @@ public class ChmItsfHeader implements
ChmAccessor<ChmItsfHeader> {
if (4 > getDataRemained()) {
throw new TikaException("4 > dataLenght");
}
+ //shift the top byte in long arithmetic so a value >= 2^31 stays
unsigned
dest = (data[this.getCurrentPlace()] & 0xff) |
(data[this.getCurrentPlace() + 1] & 0xff) << 8 |
(data[this.getCurrentPlace() + 2] & 0xff) << 16 |
- (data[this.getCurrentPlace() + 3] & 0xff) << 24;
+ ((long) (data[this.getCurrentPlace() + 3] & 0xff)) << 24;
setDataRemained(this.getDataRemained() - 4);
this.setCurrentPlace(this.getCurrentPlace() + 4);
diff --git
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmItspHeader.java
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmItspHeader.java
index c2f8eb9a53..6ede88b9d1 100644
---
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmItspHeader.java
+++
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmItspHeader.java
@@ -166,10 +166,11 @@ public class ChmItspHeader implements
ChmAccessor<ChmItspHeader> {
if (4 > dataLenght) {
throw new TikaException("4 > dataLenght");
}
+ //shift the top byte in long arithmetic so a value >= 2^31 stays
unsigned
dest = (data[this.getCurrentPlace()] & 0xff) |
(data[this.getCurrentPlace() + 1] & 0xff) << 8 |
(data[this.getCurrentPlace() + 2] & 0xff) << 16 |
- (data[this.getCurrentPlace() + 3] & 0xff) << 24;
+ ((long) (data[this.getCurrentPlace() + 3] & 0xff)) << 24;
setDataRemained(this.getDataRemained() - 4);
this.setCurrentPlace(this.getCurrentPlace() + 4);
diff --git
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmLzxBlock.java
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmLzxBlock.java
index d1cd17e758..e505c5ed63 100644
---
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmLzxBlock.java
+++
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmLzxBlock.java
@@ -876,10 +876,14 @@ public class ChmLzxBlock {
} else {
throw new ChmParsingException("data segment should not be null");
}
- if (blockLength > 0) {
+ //cap at MAX_CONTENT_SIZE: the content buffer is (int) blockLength, so
a
+ //blockLength past that either overflows the int (negative allocation)
or
+ //trips the Integer.MAX_VALUE branch in checkLzxBlock. Real blocks are
~32KB.
+ if (blockLength > 0 && blockLength <= MAX_CONTENT_SIZE) {
++goodParameter;
} else {
- throw new ChmParsingException("block length should be more than
zero");
+ throw new ChmParsingException(
+ "block length should be between 1 and " + MAX_CONTENT_SIZE
+ ": " + blockLength);
}
return (goodParameter == 3);
}
diff --git
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmLzxcControlData.java
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmLzxcControlData.java
index 1d63951ef2..9e6299ee11 100644
---
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmLzxcControlData.java
+++
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmLzxcControlData.java
@@ -225,10 +225,11 @@ public class ChmLzxcControlData implements
ChmAccessor<ChmLzxcControlData> {
if (4 > getDataRemained()) {
throw new ChmParsingException("4 > dataLenght");
}
+ //shift the top byte in long arithmetic so a value >= 2^31 stays
unsigned
dest = (data[this.getCurrentPlace()] & 0xff) |
(data[this.getCurrentPlace() + 1] & 0xff) << 8 |
(data[this.getCurrentPlace() + 2] & 0xff) << 16 |
- (data[this.getCurrentPlace() + 3] & 0xff) << 24;
+ ((long) (data[this.getCurrentPlace() + 3] & 0xff)) << 24;
setDataRemained(this.getDataRemained() - 4);
this.setCurrentPlace(this.getCurrentPlace() + 4);
diff --git
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmLzxcResetTable.java
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmLzxcResetTable.java
index 91543aa3bb..adff2e6c5b 100644
---
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmLzxcResetTable.java
+++
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmLzxcResetTable.java
@@ -110,9 +110,16 @@ public class ChmLzxcResetTable implements
ChmAccessor<ChmLzxcResetTable> {
setBlockCount(getDataRemained() / 8);
}
- long[] addresses = new long[(int) getBlockCount()];
+ int count = (int) getBlockCount();
+ if (count < 0) {
+ throw new ChmParsingException("negative chm reset table block
count: " + count);
+ }
+ long[] addresses = new long[count];
int rem = getDataRemained() / 8;
- for (int i = 0; i < rem; i++) {
+ //only read as many addresses as both the block count and the trailing
data
+ //allow, so a block count smaller than the data can't write past the
array
+ int toRead = Math.min(count, rem);
+ for (int i = 0; i < toRead; i++) {
long num = -1;
try {
@@ -144,10 +151,11 @@ public class ChmLzxcResetTable implements
ChmAccessor<ChmLzxcResetTable> {
private long unmarshalUInt32(byte[] data, long dest) throws TikaException {
ChmAssert.assertByteArrayNotNull(data);
+ //shift the top byte in long arithmetic so a value >= 2^31 stays
unsigned
dest = (data[this.getCurrentPlace()] & 0xff) |
(data[this.getCurrentPlace() + 1] & 0xff) << 8 |
(data[this.getCurrentPlace() + 2] & 0xff) << 16 |
- (data[this.getCurrentPlace() + 3] & 0xff) << 24;
+ ((long) (data[this.getCurrentPlace() + 3] & 0xff)) << 24;
setDataRemained(this.getDataRemained() - 4);
this.setCurrentPlace(this.getCurrentPlace() + 4);
diff --git
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmPmgiHeader.java
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmPmgiHeader.java
index a57e03d5bb..244b6cef10 100644
---
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmPmgiHeader.java
+++
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmPmgiHeader.java
@@ -91,10 +91,11 @@ public class ChmPmgiHeader implements
ChmAccessor<ChmPmgiHeader> {
if (4 > getDataRemained()) {
throw new ChmParsingException("4 > dataLenght");
}
+ //shift the top byte in long arithmetic so a value >= 2^31 stays
unsigned
dest = (data[this.getCurrentPlace()] & 0xff) |
(data[this.getCurrentPlace() + 1] & 0xff) << 8 |
(data[this.getCurrentPlace() + 2] & 0xff) << 16 |
- (data[this.getCurrentPlace() + 3] & 0xff) << 24;
+ ((long) (data[this.getCurrentPlace() + 3] & 0xff)) << 24;
setDataRemained(this.getDataRemained() - 4);
this.setCurrentPlace(this.getCurrentPlace() + 4);
diff --git
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmPmglHeader.java
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmPmglHeader.java
index e1e74fe175..59f3ba3130 100644
---
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmPmglHeader.java
+++
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmPmglHeader.java
@@ -133,10 +133,11 @@ public class ChmPmglHeader implements
ChmAccessor<ChmPmglHeader> {
if (4 > getDataRemained()) {
throw new ChmParsingException("4 > dataLenght");
}
+ //shift the top byte in long arithmetic so a value >= 2^31 stays
unsigned
dest = (data[this.getCurrentPlace()] & 0xff) |
(data[this.getCurrentPlace() + 1] & 0xff) << 8 |
(data[this.getCurrentPlace() + 2] & 0xff) << 16 |
- (data[this.getCurrentPlace() + 3] & 0xff) << 24;
+ ((long) (data[this.getCurrentPlace() + 3] & 0xff)) << 24;
setDataRemained(this.getDataRemained() - 4);
this.setCurrentPlace(this.getCurrentPlace() + 4);
diff --git
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmSection.java
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmSection.java
index 6c2961c170..e0cf2e86f5 100644
---
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmSection.java
+++
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/chm/ChmSection.java
@@ -161,15 +161,16 @@ public class ChmSection {
public BigInteger getEncint() {
byte ob;
- BigInteger bi = BigInteger.ZERO;
- byte[] nb = new byte[1];
+ //accumulate in a fixed-width long rather than a BigInteger: a run of
+ //continuation bytes from a hostile section would otherwise grow the
+ //BigInteger without bound, an O(n^2) memory/CPU DoS. getByte()
returns 0
+ //past the end of data, which terminates the loop.
+ long value = 0;
while ((ob = this.getByte()) < 0) {
- nb[0] = (byte) ((ob & 0x7f));
- bi = bi.shiftLeft(7).add(new BigInteger(nb));
+ value = (value << 7) | (ob & 0x7f);
}
- nb[0] = (byte) ((ob & 0x7f));
- bi = bi.shiftLeft(7).add(new BigInteger(nb));
- return bi;
+ value = (value << 7) | (ob & 0x7f);
+ return BigInteger.valueOf(value);
}
// private void setData(byte[] data) {
diff --git
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/test/java/org/apache/tika/parser/microsoft/chm/TestChmLzxcControlData.java
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/test/java/org/apache/tika/parser/microsoft/chm/TestChmLzxcControlData.java
index 277cb695ee..964f14232a 100644
---
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/test/java/org/apache/tika/parser/microsoft/chm/TestChmLzxcControlData.java
+++
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/test/java/org/apache/tika/parser/microsoft/chm/TestChmLzxcControlData.java
@@ -142,4 +142,22 @@ public class TestChmLzxcControlData {
assertEquals(0x8000L, control.getSize());
}
+ @Test
+ public void testUInt32TopBitNotSignExtended() throws Exception {
+ // size = 0x80000000 little-endian; the top byte's shift must be done
in long
+ // arithmetic, else the whole uint32 reads as a negative int
(-2147483648)
+ byte[] data = new byte[ChmConstants.CHM_LZXC_MIN_LEN];
+ data[3] = (byte) 0x80; // size = 0x80000000
+ byte[] sig = ChmConstants.LZXC.getBytes(UTF_8);
+ System.arraycopy(sig, 0, data, 4, sig.length);
+ data[8] = 0x01; // version
+ data[12] = 0x02; // resetInterval
+ data[16] = 0x02; // windowSize
+ data[20] = 0x02; // windowsPerReset
+
+ ChmLzxcControlData control = new ChmLzxcControlData();
+ control.parse(data, control);
+ assertEquals(0x80000000L, control.getSize());
+ }
+
}
diff --git
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/test/java/org/apache/tika/parser/microsoft/chm/TestChmMalformed.java
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/test/java/org/apache/tika/parser/microsoft/chm/TestChmMalformed.java
new file mode 100644
index 0000000000..ad8d10585e
--- /dev/null
+++
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/test/java/org/apache/tika/parser/microsoft/chm/TestChmMalformed.java
@@ -0,0 +1,194 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tika.parser.microsoft.chm;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.Arrays;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+
+import org.apache.tika.exception.TikaException;
+
+/**
+ * Characterization harness for CHM parsing on malformed / hostile input.
+ * <p>
+ * CHM is a binary format read from untrusted bytes. The contract for any Tika
parser is that
+ * bad input surfaces as a {@link TikaException} (here {@link
ChmParsingException}), never as a
+ * raw {@link RuntimeException} ({@code IllegalArgumentException}, {@code
NegativeArraySizeException},
+ * {@code ArrayIndexOutOfBoundsException}, {@code NullPointerException}) and
never as an
+ * {@code OutOfMemoryError}. These tests assert that target. A failing test
here documents a spot
+ * where malformed input currently escapes as the wrong throwable; the
hardening turns it green,
+ * while {@code TestChmExtraction} guards that real CHM files still parse.
+ */
+public class TestChmMalformed {
+
+ // ------------------------------------------------------------------
+ // ChmCommons.copyOfRange: the shared offset chokepoint. Malformed CHM
+ // offsets/lengths reach it as out-of-range (from, to) after being cast
+ // from parsed uint32/uint64 fields. It already declares throws
TikaException.
+ // ------------------------------------------------------------------
+
+ @Test
+ public void testCopyOfRangeToBeyondLength() {
+ byte[] data = new byte[16];
+ assertThrows(TikaException.class, () -> ChmCommons.copyOfRange(data,
0, 32));
+ }
+
+ @Test
+ public void testCopyOfRangeFromGreaterThanTo() {
+ byte[] data = new byte[16];
+ assertThrows(TikaException.class, () -> ChmCommons.copyOfRange(data,
12, 4));
+ }
+
+ @Test
+ public void testCopyOfRangeNegativeFrom() {
+ // a wrapped long->int offset can land negative
+ byte[] data = new byte[16];
+ assertThrows(TikaException.class, () -> ChmCommons.copyOfRange(data,
-1, 4));
+ }
+
+ @Test
+ public void testCopyOfRangeFromBeyondLength() {
+ byte[] data = new byte[16];
+ assertThrows(TikaException.class, () -> ChmCommons.copyOfRange(data,
32, 40));
+ }
+
+ // ------------------------------------------------------------------
+ // ChmExtractor end-to-end: the ctor parses ITSF/ITSP/directory eagerly.
+ // ------------------------------------------------------------------
+
+ @Test
+ public void testTruncatedShorterThanItsfHeader() {
+ // shorter than CHM_ITSF_V3_LEN, which the ctor immediately
copyOfRange's out
+ byte[] tiny = new byte[10];
+ assertThrows(TikaException.class,
+ () -> new ChmExtractor(new ByteArrayInputStream(tiny)));
+ }
+
+ @Test
+ public void testEmptyInput() {
+ assertThrows(TikaException.class,
+ () -> new ChmExtractor(new ByteArrayInputStream(new byte[0])));
+ }
+
+ @Test
+ public void testItsfLengthButAllZero() {
+ // long enough for the first header copy, but not a valid ITSF
signature
+ byte[] zeros = new byte[512];
+ assertThrows(TikaException.class,
+ () -> new ChmExtractor(new ByteArrayInputStream(zeros)));
+ }
+
+ @Test
+ public void testIOExceptionNotSwallowed() {
+ // a stream that fails mid-read must surface, not leave a half-built
extractor
+ // that NPEs later in ChmParser
+ InputStream broken = new InputStream() {
+ @Override
+ public int read() throws IOException {
+ throw new IOException("boom");
+ }
+ };
+ assertThrows(IOException.class, () -> new ChmExtractor(broken));
+ }
+
+ // ------------------------------------------------------------------
+ // ChmLzxcResetTable.enumerateBlockAddresses allocates new
long[blockCount] and
+ // reads dataRemained/8 addresses. A block count that is huge (uint32 high
bit)
+ // or smaller than the trailing data must not become a huge/negative
allocation
+ // or write past the array.
+ // ------------------------------------------------------------------
+
+ @Test
+ public void testResetTableHugeBlockCountIsCapped() {
+ // 40-byte header, no address data; blockCount 0x80000000 is a large
unsigned
+ // value that must be capped rather than allocated in full or read as
negative
+ byte[] data = new byte[40];
+ data[0] = 0x02; // version = 2 (little-endian)
+ data[7] = (byte) 0x80; // blockCount = 0x80000000 = 2147483648
unsigned
+ ChmLzxcResetTable table = new ChmLzxcResetTable();
+ assertDoesNotThrow(() -> table.parse(data, table));
+ assertTrue(table.getBlockAddress().length <= 5000);
+ }
+
+ @Test
+ public void testResetTableBlockCountSmallerThanData() {
+ // block count smaller than the trailing address data must not write
past
+ // the addresses array (previously an AIOOBE)
+ byte[] data = new byte[56]; // 40-byte header + room for 2 addresses
+ data[0] = 0x02; // version = 2
+ data[4] = 0x01; // blockCount = 1
+ ChmLzxcResetTable table = new ChmLzxcResetTable();
+ assertDoesNotThrow(() -> table.parse(data, table));
+ assertEquals(1, table.getBlockAddress().length);
+ }
+
+ // ------------------------------------------------------------------
+ // ChmLzxBlock allocates content of (int) blockLength. A blockLength coming
+ // from a crafted reset table that is >= Integer.MAX_VALUE overflows the
int
+ // (negative content length) and, before that, null-derefs in
checkLzxBlock.
+ // ------------------------------------------------------------------
+
+ @Test
+ public void testLzxBlockOversizeBlockLength() {
+ // blockLength >= Integer.MAX_VALUE (also > MAX_CONTENT_SIZE)
+ assertThrows(TikaException.class,
+ () -> new ChmLzxBlock(0, new byte[16], 0x80000000L, null));
+ }
+
+ // ------------------------------------------------------------------
+ // Entry name lengths are variable-length ENCINTs. A signed-byte
comparison used
+ // to cap the decoded value at 127 (corrupting longer names), and
BigInteger-based
+ // decoders grew without bound on a run of continuation bytes (an O(n^2)
DoS).
+ // ------------------------------------------------------------------
+
+ @Test
+ public void testDecodeEncintSingleByte() {
+ assertEquals(0, ChmDirectoryListingSet.decodeEncint(new byte[] {0x00},
0));
+ assertEquals(5, ChmDirectoryListingSet.decodeEncint(new byte[] {0x05},
0));
+ assertEquals(127, ChmDirectoryListingSet.decodeEncint(new byte[]
{0x7f}, 0));
+ }
+
+ @Test
+ public void testDecodeEncintMultiByte() {
+ // the high bit signals a continuation byte: 0x81 0x00 = 128, 0x81
0x48 = 200
+ assertEquals(128, ChmDirectoryListingSet.decodeEncint(new byte[]
{(byte) 0x81, 0x00}, 0));
+ assertEquals(200, ChmDirectoryListingSet.decodeEncint(new byte[]
{(byte) 0x81, 0x48}, 0));
+ assertEquals(16384,
+ ChmDirectoryListingSet.decodeEncint(new byte[] {(byte) 0x81,
(byte) 0x80, 0x00}, 0));
+ }
+
+ @Test
+ @Timeout(value = 30, unit = TimeUnit.SECONDS)
+ public void testChmSectionGetEncintNotUnboundedBigInteger() throws
Exception {
+ // a section that is all continuation bytes must not accumulate an
unbounded
+ // BigInteger (previously O(n^2)); the bounded long version returns
promptly
+ byte[] hostile = new byte[1_000_000];
+ Arrays.fill(hostile, (byte) 0xff);
+ ChmSection section = new ChmSection(hostile);
+ assertDoesNotThrow(section::getEncint);
+ }
+}
diff --git
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/test/java/org/apache/tika/parser/microsoft/chm/TestParameters.java
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/test/java/org/apache/tika/parser/microsoft/chm/TestParameters.java
index 2774ce56f7..da742fd778 100644
---
a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/test/java/org/apache/tika/parser/microsoft/chm/TestParameters.java
+++
b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/test/java/org/apache/tika/parser/microsoft/chm/TestParameters.java
@@ -57,7 +57,8 @@ public class TestParameters {
static final int VP_INDEX_DEPTH = 1;
static final int VP_INDEX_HEAD = 0;
static final int VP_INDEX_ROOT = -1;
- static final int VP_UNKNOWN_NUM_BLOCKS = -1;
+ //0xFFFFFFFF read as an unsigned uint32 (was -1 before the sign-extension
fix)
+ static final long VP_UNKNOWN_NUM_BLOCKS = 0xFFFFFFFFL;
static final int VP_ITSP_UNKNOWN_000C = 10;
static final int VP_ITSP_UNKNOWN_0024 = 0;
static final int VP_ITSP_UNKNOWN_002C = 1;
