From 0402cb32df015d9372578e3db27db47b33d5c7b0 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sun, 22 Oct 2017 18:23:23 +0200 Subject: bunzip2: fix runCnt overflow from bug 10431 This particular corrupted file can be dealth with by using "unsigned". If there will be cases where it genuinely overflows, there is a disabled code to deal with that too. function old new delta get_next_block 1678 1667 -11 Signed-off-by: Denys Vlasenko --- archival/libarchive/decompress_bunzip2.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) (limited to 'archival/libarchive') diff --git a/archival/libarchive/decompress_bunzip2.c b/archival/libarchive/decompress_bunzip2.c index 7cd18f5ed..bec89edd3 100644 --- a/archival/libarchive/decompress_bunzip2.c +++ b/archival/libarchive/decompress_bunzip2.c @@ -156,15 +156,15 @@ static unsigned get_bits(bunzip_data *bd, int bits_wanted) static int get_next_block(bunzip_data *bd) { struct group_data *hufGroup; - int dbufCount, dbufSize, groupCount, *base, *limit, selector, - i, j, runPos, symCount, symTotal, nSelectors, byteCount[256]; - int runCnt = runCnt; /* for compiler */ + int groupCount, *base, *limit, selector, + i, j, symCount, symTotal, nSelectors, byteCount[256]; uint8_t uc, symToByte[256], mtfSymbol[256], *selectors; uint32_t *dbuf; unsigned origPtr, t; + unsigned dbufCount, runPos; + unsigned runCnt = runCnt; /* for compiler */ dbuf = bd->dbuf; - dbufSize = bd->dbufSize; selectors = bd->selectors; /* In bbox, we are ok with aborting through setjmp which is set up in start_bunzip */ @@ -187,7 +187,7 @@ static int get_next_block(bunzip_data *bd) it didn't actually work. */ if (get_bits(bd, 1)) return RETVAL_OBSOLETE_INPUT; origPtr = get_bits(bd, 24); - if ((int)origPtr > dbufSize) return RETVAL_DATA_ERROR; + if (origPtr > bd->dbufSize) return RETVAL_DATA_ERROR; /* mapping table: if some byte values are never used (encoding things like ascii text), the compression code removes the gaps to have fewer @@ -435,7 +435,14 @@ static int get_next_block(bunzip_data *bd) symbols, but a run of length 0 doesn't mean anything in this context). Thus space is saved. */ runCnt += (runPos << nextSym); /* +runPos if RUNA; +2*runPos if RUNB */ - if (runPos < dbufSize) runPos <<= 1; +//The 32-bit overflow of runCnt wasn't yet seen, but probably can happen. +//This would be the fix (catches too large count way before it can overflow): +// if (runCnt > bd->dbufSize) { +// dbg("runCnt:%u > dbufSize:%u RETVAL_DATA_ERROR", +// runCnt, bd->dbufSize); +// return RETVAL_DATA_ERROR; +// } + if (runPos < bd->dbufSize) runPos <<= 1; goto end_of_huffman_loop; } @@ -445,14 +452,15 @@ static int get_next_block(bunzip_data *bd) literal used is the one at the head of the mtfSymbol array.) */ if (runPos != 0) { uint8_t tmp_byte; - if (dbufCount + runCnt > dbufSize) { - dbg("dbufCount:%d+runCnt:%d %d > dbufSize:%d RETVAL_DATA_ERROR", - dbufCount, runCnt, dbufCount + runCnt, dbufSize); + if (dbufCount + runCnt > bd->dbufSize) { + dbg("dbufCount:%u+runCnt:%u %u > dbufSize:%u RETVAL_DATA_ERROR", + dbufCount, runCnt, dbufCount + runCnt, bd->dbufSize); return RETVAL_DATA_ERROR; } tmp_byte = symToByte[mtfSymbol[0]]; byteCount[tmp_byte] += runCnt; - while (--runCnt >= 0) dbuf[dbufCount++] = (uint32_t)tmp_byte; + while ((int)--runCnt >= 0) + dbuf[dbufCount++] = (uint32_t)tmp_byte; runPos = 0; } @@ -466,7 +474,7 @@ static int get_next_block(bunzip_data *bd) first symbol in the mtf array, position 0, would have been handled as part of a run above. Therefore 1 unused mtf position minus 2 non-literal nextSym values equals -1.) */ - if (dbufCount >= dbufSize) return RETVAL_DATA_ERROR; + if (dbufCount >= bd->dbufSize) return RETVAL_DATA_ERROR; i = nextSym - 1; uc = mtfSymbol[i]; -- cgit v1.2.3