emo/busybox
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bunzip2: fix runCnt overflow from bug 10431

Browse Source 
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>
This commit is contained in:
Denys Vlasenko 2017-10-22 18:23:23 +02:00
parent 25f3b737dc
commit 0402cb32df
1 changed files with 19 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
archival/libarchive/decompress_bunzip2.c
View File

@ -156,15 +156,15 @@ static unsigned get_bits(bunzip_data *bd, int bits_wanted)
static int get_next_block(bunzip_data *bd) static int get_next_block(bunzip_data *bd)
{ {
struct group_data *hufGroup; struct group_data *hufGroup;
int dbufCount, dbufSize, groupCount, *base, *limit, selector, int groupCount, *base, *limit, selector,
i, j, runPos, symCount, symTotal, nSelectors, byteCount[256]; i, j, symCount, symTotal, nSelectors, byteCount[256];
int runCnt = runCnt; /* for compiler */
uint8_t uc, symToByte[256], mtfSymbol[256], *selectors; uint8_t uc, symToByte[256], mtfSymbol[256], *selectors;
uint32_t *dbuf; uint32_t *dbuf;
unsigned origPtr, t; unsigned origPtr, t;
unsigned dbufCount, runPos;
unsigned runCnt = runCnt; /* for compiler */
dbuf = bd->dbuf; dbuf = bd->dbuf;
dbufSize = bd->dbufSize;
selectors = bd->selectors; selectors = bd->selectors;
/* In bbox, we are ok with aborting through setjmp which is set up in start_bunzip */ /* 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. */ it didn't actually work. */
if (get_bits(bd, 1)) return RETVAL_OBSOLETE_INPUT; if (get_bits(bd, 1)) return RETVAL_OBSOLETE_INPUT;
origPtr = get_bits(bd, 24); 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 /* mapping table: if some byte values are never used (encoding things
like ascii text), the compression code removes the gaps to have fewer 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 symbols, but a run of length 0 doesn't mean anything in this
context). Thus space is saved. */ context). Thus space is saved. */
runCnt += (runPos << nextSym); /* +runPos if RUNA; +2*runPos if RUNB */ 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; 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.) */ literal used is the one at the head of the mtfSymbol array.) */
if (runPos != 0) { if (runPos != 0) {
uint8_t tmp_byte; uint8_t tmp_byte;
if (dbufCount + runCnt > dbufSize) { if (dbufCount + runCnt > bd->dbufSize) {
dbg("dbufCount:%d+runCnt:%d %d > dbufSize:%d RETVAL_DATA_ERROR", dbg("dbufCount:%u+runCnt:%u %u > dbufSize:%u RETVAL_DATA_ERROR",
dbufCount, runCnt, dbufCount + runCnt, dbufSize); dbufCount, runCnt, dbufCount + runCnt, bd->dbufSize);
return RETVAL_DATA_ERROR; return RETVAL_DATA_ERROR;
} }
tmp_byte = symToByte[mtfSymbol[0]]; tmp_byte = symToByte[mtfSymbol[0]];
byteCount[tmp_byte] += runCnt; 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; 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 first symbol in the mtf array, position 0, would have been handled
as part of a run above. Therefore 1 unused mtf position minus as part of a run above. Therefore 1 unused mtf position minus
2 non-literal nextSym values equals -1.) */ 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; i = nextSym - 1;
uc = mtfSymbol[i]; uc = mtfSymbol[i];