From 6a8cbf1548cf0a02ca1adfb3294b4a1f4a23b5ba Mon Sep 17 00:00:00 2001 From: Sebastião Guerra Date: Sun, 20 Sep 2026 08:54:57 -0300 Subject: KPDF: Fix JBIG2 segment accounting on encrypted streams MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit JBIG2 segment-length accounting used Stream::getPos() to determine how many bytes of a segment had been consumed. With AES-encrypted streams, DecryptStream inherits the underlying encrypted stream position, which does not necessarily correspond to the number of plaintext bytes already returned because AES decryption is block-buffered. This could make JBIG2Stream discard valid plaintext bytes at the end of a segment. Subsequent segment headers were then parsed at the wrong offset, eventually producing invalid symbol counts and "Bogus memory allocation size" errors. Track plaintext bytes consumed by the arithmetic, Huffman and MMR decoders, as well as direct JBIG2 stream reads, and use those counters for segment accounting instead of Stream::getPos(). Ensure truncated collective-bitmap reads clean up the temporary bitmap before entering the existing symbol-dictionary error cleanup, avoiding leaks on malformed JBIG2 data. Also avoid reporting an encoded-stream position as a JBIG2 plaintext position. The JBIG2 plaintext byte-accounting implementation is selectively adapted and strongly based on Xpdf 3.04: Project: Xpdf Version: 3.04 Files: xpdf/JArithmeticDecoder.cc, xpdf/JArithmeticDecoder.h, xpdf/JBIG2Stream.cc, xpdf/JBIG2Stream.h Source: https://dl.xpdfreader.com/old/xpdf-3.04.tar.gz The adapted portions include arithmetic, Huffman and MMR decoder byte counters and their use for JBIG2 segment accounting. Because the embedded Xpdf 3.02 Stream API lacks Xpdf 3.04's discardChars() and getBlock() helpers, equivalent local helper/read-loop implementations were added. Existing Glyph & Cog copyright and license notices are retained. Closes #49 Signed-off-by: Sebastião Guerra --- kpdf/xpdf/xpdf/JArithmeticDecoder.cpp | 3 + kpdf/xpdf/xpdf/JArithmeticDecoder.h | 4 + kpdf/xpdf/xpdf/JBIG2Stream.cpp | 152 +++++++++++++++++++--------------- kpdf/xpdf/xpdf/JBIG2Stream.h | 2 + 4 files changed, 95 insertions(+), 66 deletions(-) diff --git a/kpdf/xpdf/xpdf/JArithmeticDecoder.cpp b/kpdf/xpdf/xpdf/JArithmeticDecoder.cpp index 7f7c7685..fccc05e9 100644 --- a/kpdf/xpdf/xpdf/JArithmeticDecoder.cpp +++ b/kpdf/xpdf/xpdf/JArithmeticDecoder.cpp @@ -89,6 +89,7 @@ int JArithmeticDecoder::switchTab[47] = { JArithmeticDecoder::JArithmeticDecoder() { str = NULL; + nBytesRead = 0; dataLen = 0; limitStream = gFalse; } @@ -100,6 +101,8 @@ inline Guint JArithmeticDecoder::readByte() { return 0xff; } } + // Consumers such as JBIG2 use this to account for consumed bytes. + ++nBytesRead; return (Guint)str->getChar() & 0xff; } diff --git a/kpdf/xpdf/xpdf/JArithmeticDecoder.h b/kpdf/xpdf/xpdf/JArithmeticDecoder.h index a40823dd..f26cd03a 100644 --- a/kpdf/xpdf/xpdf/JArithmeticDecoder.h +++ b/kpdf/xpdf/xpdf/JArithmeticDecoder.h @@ -84,6 +84,9 @@ public: Guint decodeIAID(Guint codeLen, JArithmeticDecoderStats *stats); + void resetByteCounter() { nBytesRead = 0; } + Guint getByteCounter() const { return nBytesRead; } + private: Guint readByte(); @@ -102,6 +105,7 @@ private: Guint prev; // for the integer decoder Stream *str; + Guint nBytesRead; int dataLen; GBool limitStream; }; diff --git a/kpdf/xpdf/xpdf/JBIG2Stream.cpp b/kpdf/xpdf/xpdf/JBIG2Stream.cpp index 1a4058f8..56216908 100644 --- a/kpdf/xpdf/xpdf/JBIG2Stream.cpp +++ b/kpdf/xpdf/xpdf/JBIG2Stream.cpp @@ -305,6 +305,9 @@ public: Guint readBits(Guint n); Guint readBit(); + void resetByteCounter() { byteCounter = 0; } + Guint getByteCounter() const { return byteCounter; } + // Sort the table by prefix length and assign prefix values. void buildTable(JBIG2HuffmanTable *table, Guint len); @@ -313,10 +316,12 @@ private: Stream *str; Guint buf; Guint bufLen; + Guint byteCounter; }; JBIG2HuffmanDecoder::JBIG2HuffmanDecoder() { str = NULL; + byteCounter = 0; reset(); } @@ -371,10 +376,12 @@ Guint JBIG2HuffmanDecoder::readBits(Guint n) { bufLen = 0; while (nLeft >= 8) { x = (x << 8) | (str->getChar() & 0xff); + ++byteCounter; nLeft -= 8; } if (nLeft > 0) { buf = str->getChar(); + ++byteCounter; bufLen = 8 - nLeft; x = (x << nLeft) | ((buf >> bufLen) & ((1 << nLeft) - 1)); } @@ -385,6 +392,7 @@ Guint JBIG2HuffmanDecoder::readBits(Guint n) { Guint JBIG2HuffmanDecoder::readBit() { if (bufLen == 0) { buf = str->getChar(); + ++byteCounter; bufLen = 8; } --bufLen; @@ -448,6 +456,8 @@ public: int getBlackCode(); int getWhiteCode(); Guint get24Bits(); + void resetByteCounter() { byteCounter = 0; } + Guint getByteCounter() const { return byteCounter; } void skipTo(Guint length); private: @@ -456,10 +466,12 @@ private: Guint buf; Guint bufLen; Guint nBytesRead; + Guint byteCounter; }; JBIG2MMRDecoder::JBIG2MMRDecoder() { str = NULL; + byteCounter = 0; reset(); } @@ -479,6 +491,7 @@ int JBIG2MMRDecoder::get2DCode() { buf = str->getChar() & 0xff; bufLen = 8; ++nBytesRead; + ++byteCounter; p = &twoDimTab1[(buf >> 1) & 0x7f]; } else if (bufLen == 8) { p = &twoDimTab1[(buf >> 1) & 0x7f]; @@ -488,6 +501,7 @@ int JBIG2MMRDecoder::get2DCode() { buf = (buf << 8) | (str->getChar() & 0xff); bufLen += 8; ++nBytesRead; + ++byteCounter; p = &twoDimTab1[(buf >> (bufLen - 7)) & 0x7f]; } } @@ -507,6 +521,7 @@ int JBIG2MMRDecoder::getWhiteCode() { buf = str->getChar() & 0xff; bufLen = 8; ++nBytesRead; + ++byteCounter; } while (1) { if (bufLen >= 11 && ((buf >> (bufLen - 7)) & 0x7f) == 0) { @@ -534,6 +549,7 @@ int JBIG2MMRDecoder::getWhiteCode() { buf = (buf << 8) | (str->getChar() & 0xff); bufLen += 8; ++nBytesRead; + ++byteCounter; } error(str->getPos(), "Bad white code in JBIG2 MMR stream"); // eat a bit and return a positive number so that the caller doesn't @@ -550,6 +566,7 @@ int JBIG2MMRDecoder::getBlackCode() { buf = str->getChar() & 0xff; bufLen = 8; ++nBytesRead; + ++byteCounter; } while (1) { if (bufLen >= 10 && ((buf >> (bufLen - 6)) & 0x3f) == 0) { @@ -585,6 +602,7 @@ int JBIG2MMRDecoder::getBlackCode() { buf = (buf << 8) | (str->getChar() & 0xff); bufLen += 8; ++nBytesRead; + ++byteCounter; } error(str->getPos(), "Bad black code in JBIG2 MMR stream"); // eat a bit and return a positive number so that the caller doesn't @@ -598,14 +616,21 @@ Guint JBIG2MMRDecoder::get24Bits() { buf = (buf << 8) | (str->getChar() & 0xff); bufLen += 8; ++nBytesRead; + ++byteCounter; } return (buf >> (bufLen - 24)) & 0xffffff; } void JBIG2MMRDecoder::skipTo(Guint length) { - while (nBytesRead < length) { - str->getChar(); - ++nBytesRead; + Guint n; + + if (nBytesRead < length) { + n = length - nBytesRead; + while (n > 0 && str->getChar() != EOF) { + --n; + ++nBytesRead; + ++byteCounter; + } } } @@ -1146,6 +1171,7 @@ JBIG2Stream::JBIG2Stream(Stream *strA, Object *globalsStreamA): segments = globalSegments = NULL; curStr = NULL; dataPtr = dataEnd = NULL; + byteCounter = 0; } JBIG2Stream::~JBIG2Stream() { @@ -1247,7 +1273,6 @@ void JBIG2Stream::readSegments() { Guint segNum, segFlags, segType, page, segLength; Guint refFlags, nRefSegs; Guint *refSegs; - int segDataPos; int c1, c2, c3; Guint i; @@ -1272,8 +1297,9 @@ void JBIG2Stream::readSegments() { } refFlags = (refFlags << 24) | (c1 << 16) | (c2 << 8) | c3; nRefSegs = refFlags & 0x1fffffff; - for (i = 0; i < (nRefSegs + 9) >> 3; ++i) { - c1 = curStr->getChar(); + i = (nRefSegs + 9) >> 3; + if (discardChars(i) != i) { + goto eofError1; } } @@ -1315,9 +1341,6 @@ void JBIG2Stream::readSegments() { goto eofError2; } - // keep track of the start of the segment data - segDataPos = getPos(); - // check for missing page information segment if (!pageBitmap && ((segType >= 4 && segType <= 7) || (segType >= 20 && segType <= 43))) { @@ -1326,6 +1349,10 @@ void JBIG2Stream::readSegments() { } // read the segment data + arithDecoder->resetByteCounter(); + huffDecoder->resetByteCounter(); + mmrDecoder->resetByteCounter(); + byteCounter = 0; switch (segType) { case 0: if (!readSymbolDictSeg(segNum, segLength, refSegs, nRefSegs)) { @@ -1394,53 +1421,31 @@ void JBIG2Stream::readSegments() { break; default: error(getPos(), "Unknown segment type in JBIG2 stream"); - for (i = 0; i < segLength; ++i) { - if ((c1 = curStr->getChar()) == EOF) { - goto eofError2; - } + byteCounter += discardChars(segLength); + if (byteCounter != segLength) { + goto eofError2; } break; } - // Make sure the segment handler read all of the bytes in the - // segment data, unless this segment is marked as having an - // unknown length (section 7.2.7 of the JBIG2 Final Committee Draft) - - if (segLength != 0xffffffff) { - - int segExtraBytes = segDataPos + segLength - getPos(); - if (segExtraBytes > 0) { - - // If we didn't read all of the bytes in the segment data, - // indicate an error, and throw away the rest of the data. - - // v.3.1.01.13 of the LuraTech PDF Compressor Server will - // sometimes generate an extraneous NULL byte at the end of - // arithmetic-coded symbol dictionary segments when numNewSyms - // == 0. Segments like this often occur for blank pages. - - error(getPos(), "%d extraneous byte%s after segment", - segExtraBytes, (segExtraBytes > 1) ? "s" : ""); - - // Burn through the remaining bytes -- inefficient, but - // hopefully we're not doing this much - - int trash; - for (int i = segExtraBytes; i > 0; i--) { - readByte(&trash); - } - - } else if (segExtraBytes < 0) { - - // If we read more bytes than we should have, according to the - // segment length field, note an error. - - error(getPos(), "Previous segment handler read too many bytes"); - + // skip any unused data at the end of the segment + // (except for immediate generic region segments which have + // 0xffffffff = unspecified length) + if (!(segType == 38 && segLength == 0xffffffff)) { + byteCounter += arithDecoder->getByteCounter(); + byteCounter += huffDecoder->getByteCounter(); + byteCounter += mmrDecoder->getByteCounter(); + // do a sanity check on byteCounter vs segLength -- if there is + // a problem, abort the decode + if (byteCounter > segLength || + segLength - byteCounter > 65536) { + error(-1, "Invalid segment length in JBIG2 stream"); + gfree(refSegs); + break; } - + byteCounter += discardChars(segLength - byteCounter); } - + gfree(refSegs); } @@ -1456,6 +1461,16 @@ void JBIG2Stream::readSegments() { error(getPos(), "Unexpected EOF in JBIG2 stream"); } +Guint JBIG2Stream::discardChars(Guint n) { + Guint count; + + count = 0; + while (count < n && curStr->getChar() != EOF) { + ++count; + } + return count; +} + GBool JBIG2Stream::readSymbolDictSeg(Guint segNum, Guint /*length*/, Guint *refSegs, Guint nRefSegs) { JBIG2SymbolDict *symbolDict; @@ -1474,6 +1489,8 @@ GBool JBIG2Stream::readSymbolDictSeg(Guint segNum, Guint /*length*/, Guint *symWidths; Guint symHeight, symWidth, totalWidth, x, symID; int dh, dw, refAggNum, refDX, refDY, bmSize; + int c; + GBool eof; GBool ex; int run, cnt; Guint i, j, k; @@ -1776,8 +1793,18 @@ GBool JBIG2Stream::readSymbolDictSeg(Guint segNum, Guint /*length*/, collBitmap = new JBIG2Bitmap(0, totalWidth, symHeight); bmSize = symHeight * ((totalWidth + 7) >> 3); p = collBitmap->getDataPtr(); + eof = gFalse; for (k = 0; k < (Guint)bmSize; ++k) { - *p++ = curStr->getChar(); + if ((c = curStr->getChar()) == EOF) { + eof = gTrue; + break; + } + *p++ = (Guchar)c; + ++byteCounter; + } + if (eof) { + delete collBitmap; + goto syntaxError; } } else { collBitmap = readGenericBitmap(gTrue, totalWidth, symHeight, @@ -3389,21 +3416,13 @@ void JBIG2Stream::readPageInfoSeg(Guint /*length*/) { } void JBIG2Stream::readEndOfStripeSeg(Guint length) { - Guint i; - // skip the segment - for (i = 0; i < length; ++i) { - curStr->getChar(); - } + byteCounter += discardChars(length); } void JBIG2Stream::readProfilesSeg(Guint length) { - Guint i; - // skip the segment - for (i = 0; i < length; ++i) { - curStr->getChar(); - } + byteCounter += discardChars(length); } void JBIG2Stream::readCodeTableSeg(Guint segNum, Guint /*length*/) { @@ -3471,12 +3490,8 @@ void JBIG2Stream::readCodeTableSeg(Guint segNum, Guint /*length*/) { } void JBIG2Stream::readExtensionSeg(Guint length) { - Guint i; - // skip the segment - for (i = 0; i < length; ++i) { - curStr->getChar(); - } + byteCounter += discardChars(length); } JBIG2Segment *JBIG2Stream::findSegment(Guint segNum) { @@ -3590,6 +3605,7 @@ GBool JBIG2Stream::readUByte(Guint *x) { if ((c0 = curStr->getChar()) == EOF) { return gFalse; } + ++byteCounter; *x = (Guint)c0; return gTrue; } @@ -3600,6 +3616,7 @@ GBool JBIG2Stream::readByte(int *x) { if ((c0 = curStr->getChar()) == EOF) { return gFalse; } + ++byteCounter; *x = c0; if (c0 & 0x80) { *x |= -1 - 0xff; @@ -3614,6 +3631,7 @@ GBool JBIG2Stream::readUWord(Guint *x) { (c1 = curStr->getChar()) == EOF) { return gFalse; } + byteCounter += 2; *x = (Guint)((c0 << 8) | c1); return gTrue; } @@ -3627,6 +3645,7 @@ GBool JBIG2Stream::readULong(Guint *x) { (c3 = curStr->getChar()) == EOF) { return gFalse; } + byteCounter += 4; *x = (Guint)((c0 << 24) | (c1 << 16) | (c2 << 8) | c3); return gTrue; } @@ -3640,6 +3659,7 @@ GBool JBIG2Stream::readLong(int *x) { (c3 = curStr->getChar()) == EOF) { return gFalse; } + byteCounter += 4; *x = ((c0 << 24) | (c1 << 16) | (c2 << 8) | c3); if (c0 & 0x80) { *x |= -1 - (int)0xffffffff; diff --git a/kpdf/xpdf/xpdf/JBIG2Stream.h b/kpdf/xpdf/xpdf/JBIG2Stream.h index f3443b3d..72aea619 100644 --- a/kpdf/xpdf/xpdf/JBIG2Stream.h +++ b/kpdf/xpdf/xpdf/JBIG2Stream.h @@ -103,6 +103,7 @@ private: void readExtensionSeg(Guint length); JBIG2Segment *findSegment(Guint segNum); void discardSegment(Guint segNum); + Guint discardChars(Guint n); void resetGenericStats(Guint templ, JArithmeticDecoderStats *prevStats); void resetRefinementStats(Guint templ, @@ -124,6 +125,7 @@ private: Stream *curStr; Guchar *dataPtr; Guchar *dataEnd; + Guint byteCounter; JArithmeticDecoder *arithDecoder; JArithmeticDecoderStats *genericRegionStats; -- cgit v1.2.3