summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMark Wielaard <mark@klomp.org>2019-07-03 01:28:11 +0200
committerMark Wielaard <mark@klomp.org>2019-07-09 23:29:44 +0200
commitb07b105d1b66e32760095e3602261738443b9e13 (patch)
tree186e47e404d16a7492ba1c26d23c8988961a5756
parentFix a 'not a normal file' error when compressing large files. (diff)
downloadbzip2-b07b105d1b66e32760095e3602261738443b9e13.tar.gz
bzip2-b07b105d1b66e32760095e3602261738443b9e13.tar.bz2
bzip2-b07b105d1b66e32760095e3602261738443b9e13.tar.xz
Accept as many selectors as the file format allows.
But ignore any larger than the theoretical maximum, BZ_MAX_SELECTORS. The theoretical maximum number of selectors depends on the maximum blocksize (900000 bytes) and the number of symbols (50) that can be encoded with a different Huffman tree. BZ_MAX_SELECTORS is 18002. But the bzip2 file format allows the number of selectors to be encoded with 15 bits (because 18002 isn't a factor of 2 and doesn't fit in 14 bits). So the file format maximum is 32767 selectors. Some bzip2 encoders might actually have written out more selectors than the theoretical maximum because they rounded up the number of selectors to some convenient factor of 8. The extra 14766 selectors can never be validly used by the decompression algorithm. So we can read them, but then discard them. This is effectively what was done (by accident) before we added a check for nSelectors to be at most BZ_MAX_SELECTORS to mitigate CVE-2019-12900. The extra selectors were written out after the array inside the EState struct. But the struct has extra space allocated after the selector arrays of 18060 bytes (which is larger than 14766). All of which will be initialized later (so the overwrite of that space with extra selector values would have been harmless).
-rw-r--r--compress.c2
-rw-r--r--decompress.c10
2 files changed, 9 insertions, 3 deletions
diff --git a/compress.c b/compress.c
index 237620d..76adee6 100644
--- a/compress.c
+++ b/compress.c
@@ -454,7 +454,7 @@ void sendMTFValues ( EState* s )
454 454
455 AssertH( nGroups < 8, 3002 ); 455 AssertH( nGroups < 8, 3002 );
456 AssertH( nSelectors < 32768 && 456 AssertH( nSelectors < 32768 &&
457 nSelectors <= (2 + (900000 / BZ_G_SIZE)), 457 nSelectors <= BZ_MAX_SELECTORS,
458 3003 ); 458 3003 );
459 459
460 460
diff --git a/decompress.c b/decompress.c
index 20ce493..3303499 100644
--- a/decompress.c
+++ b/decompress.c
@@ -287,7 +287,7 @@ Int32 BZ2_decompress ( DState* s )
287 GET_BITS(BZ_X_SELECTOR_1, nGroups, 3); 287 GET_BITS(BZ_X_SELECTOR_1, nGroups, 3);
288 if (nGroups < 2 || nGroups > BZ_N_GROUPS) RETURN(BZ_DATA_ERROR); 288 if (nGroups < 2 || nGroups > BZ_N_GROUPS) RETURN(BZ_DATA_ERROR);
289 GET_BITS(BZ_X_SELECTOR_2, nSelectors, 15); 289 GET_BITS(BZ_X_SELECTOR_2, nSelectors, 15);
290 if (nSelectors < 1 || nSelectors > BZ_MAX_SELECTORS) RETURN(BZ_DATA_ERROR); 290 if (nSelectors < 1) RETURN(BZ_DATA_ERROR);
291 for (i = 0; i < nSelectors; i++) { 291 for (i = 0; i < nSelectors; i++) {
292 j = 0; 292 j = 0;
293 while (True) { 293 while (True) {
@@ -296,8 +296,14 @@ Int32 BZ2_decompress ( DState* s )
296 j++; 296 j++;
297 if (j >= nGroups) RETURN(BZ_DATA_ERROR); 297 if (j >= nGroups) RETURN(BZ_DATA_ERROR);
298 } 298 }
299 s->selectorMtf[i] = j; 299 /* Having more than BZ_MAX_SELECTORS doesn't make much sense
300 since they will never be used, but some implementations might
301 "round up" the number of selectors, so just ignore those. */
302 if (i < BZ_MAX_SELECTORS)
303 s->selectorMtf[i] = j;
300 } 304 }
305 if (nSelectors > BZ_MAX_SELECTORS)
306 nSelectors = BZ_MAX_SELECTORS;
301 307
302 /*--- Undo the MTF values for the selectors. ---*/ 308 /*--- Undo the MTF values for the selectors. ---*/
303 { 309 {