This is an automated email from the ASF dual-hosted git repository. tballison pushed a commit to branch TIKA-4783-improve-chm in repository https://gitbox.apache.org/repos/asf/tika.git
commit 30466b8ee0148bb896da06374009f3b85608e94f Author: tallison <[email protected]> AuthorDate: Fri Jul 10 10:01:43 2026 -0400 TIKA-4783 -- improve chm parsing --- .../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;
