Skip to content

Commit

Permalink
bunzip2: fix runCnt overflow from bug 10431
Browse files Browse the repository at this point in the history
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 <vda.linux@googlemail.com>
  • Loading branch information
Denys Vlasenko committed Oct 22, 2017
1 parent 25f3b73 commit 0402cb3
Showing 1 changed file with 19 additions and 11 deletions.
30 changes: 19 additions & 11 deletions archival/libarchive/decompress_bunzip2.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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];

Expand Down

0 comments on commit 0402cb3

Please sign in to comment.