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 <vda.linux@googlemail.com>

diff --git a/archival/libarchive/decompress_bunzip2.c b/archival/libarchive/decompress_bunzip2.c
index 7cd18f5ed4cfb7c01e2df0b6236dae570e270e8f..bec89edd3a4df217e8d64833f0fb346835aec6b3 100644 (file)
--- 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];