| Brad Bishop | 316dfdd | 2018-06-25 12:45:53 -0400 | [diff] [blame] | 1 | busybox-1.27.2: Fix CVE-2017-15873 |
| 2 | |
| 3 | [No upstream tracking] -- https://bugs.busybox.net/show_bug.cgi?id=10431 |
| 4 | |
| 5 | bunzip2: fix runCnt overflow |
| 6 | |
| 7 | The get_next_block function in archival/libarchive/decompress_bunzip2.c |
| 8 | in BusyBox 1.27.2 has an Integer Overflow that may lead to a write |
| 9 | access violation. |
| 10 | |
| 11 | Upstream-Status: Backport [https://git.busybox.net/busybox/commit/?id=0402cb32df015d9372578e3db27db47b33d5c7b0] |
| 12 | CVE: CVE-2017-15873 |
| 13 | bug: 10431 |
| 14 | Signed-off-by: Radovan Scasny <radovan.scasny@siemens.com> |
| 15 | |
| 16 | diff --git a/archival/libarchive/decompress_bunzip2.c b/archival/libarchive/decompress_bunzip2.c |
| 17 | index 7cd18f5..bec89ed 100644 |
| 18 | --- a/archival/libarchive/decompress_bunzip2.c |
| 19 | +++ b/archival/libarchive/decompress_bunzip2.c |
| 20 | @@ -156,15 +156,15 @@ static unsigned get_bits(bunzip_data *bd, int bits_wanted) |
| 21 | static int get_next_block(bunzip_data *bd) |
| 22 | { |
| 23 | struct group_data *hufGroup; |
| 24 | - int dbufCount, dbufSize, groupCount, *base, *limit, selector, |
| 25 | - i, j, runPos, symCount, symTotal, nSelectors, byteCount[256]; |
| 26 | - int runCnt = runCnt; /* for compiler */ |
| 27 | + int groupCount, *base, *limit, selector, |
| 28 | + i, j, symCount, symTotal, nSelectors, byteCount[256]; |
| 29 | uint8_t uc, symToByte[256], mtfSymbol[256], *selectors; |
| 30 | uint32_t *dbuf; |
| 31 | unsigned origPtr, t; |
| 32 | + unsigned dbufCount, runPos; |
| 33 | + unsigned runCnt = runCnt; /* for compiler */ |
| 34 | |
| 35 | dbuf = bd->dbuf; |
| 36 | - dbufSize = bd->dbufSize; |
| 37 | selectors = bd->selectors; |
| 38 | |
| 39 | /* In bbox, we are ok with aborting through setjmp which is set up in start_bunzip */ |
| 40 | @@ -187,7 +187,7 @@ static int get_next_block(bunzip_data *bd) |
| 41 | it didn't actually work. */ |
| 42 | if (get_bits(bd, 1)) return RETVAL_OBSOLETE_INPUT; |
| 43 | origPtr = get_bits(bd, 24); |
| 44 | - if ((int)origPtr > dbufSize) return RETVAL_DATA_ERROR; |
| 45 | + if (origPtr > bd->dbufSize) return RETVAL_DATA_ERROR; |
| 46 | |
| 47 | /* mapping table: if some byte values are never used (encoding things |
| 48 | like ascii text), the compression code removes the gaps to have fewer |
| 49 | @@ -435,7 +435,14 @@ static int get_next_block(bunzip_data *bd) |
| 50 | symbols, but a run of length 0 doesn't mean anything in this |
| 51 | context). Thus space is saved. */ |
| 52 | runCnt += (runPos << nextSym); /* +runPos if RUNA; +2*runPos if RUNB */ |
| 53 | - if (runPos < dbufSize) runPos <<= 1; |
| 54 | +//The 32-bit overflow of runCnt wasn't yet seen, but probably can happen. |
| 55 | +//This would be the fix (catches too large count way before it can overflow): |
| 56 | +// if (runCnt > bd->dbufSize) { |
| 57 | +// dbg("runCnt:%u > dbufSize:%u RETVAL_DATA_ERROR", |
| 58 | +// runCnt, bd->dbufSize); |
| 59 | +// return RETVAL_DATA_ERROR; |
| 60 | +// } |
| 61 | + if (runPos < bd->dbufSize) runPos <<= 1; |
| 62 | goto end_of_huffman_loop; |
| 63 | } |
| 64 | |
| 65 | @@ -445,14 +452,15 @@ static int get_next_block(bunzip_data *bd) |
| 66 | literal used is the one at the head of the mtfSymbol array.) */ |
| 67 | if (runPos != 0) { |
| 68 | uint8_t tmp_byte; |
| 69 | - if (dbufCount + runCnt > dbufSize) { |
| 70 | - dbg("dbufCount:%d+runCnt:%d %d > dbufSize:%d RETVAL_DATA_ERROR", |
| 71 | - dbufCount, runCnt, dbufCount + runCnt, dbufSize); |
| 72 | + if (dbufCount + runCnt > bd->dbufSize) { |
| 73 | + dbg("dbufCount:%u+runCnt:%u %u > dbufSize:%u RETVAL_DATA_ERROR", |
| 74 | + dbufCount, runCnt, dbufCount + runCnt, bd->dbufSize); |
| 75 | return RETVAL_DATA_ERROR; |
| 76 | } |
| 77 | tmp_byte = symToByte[mtfSymbol[0]]; |
| 78 | byteCount[tmp_byte] += runCnt; |
| 79 | - while (--runCnt >= 0) dbuf[dbufCount++] = (uint32_t)tmp_byte; |
| 80 | + while ((int)--runCnt >= 0) |
| 81 | + dbuf[dbufCount++] = (uint32_t)tmp_byte; |
| 82 | runPos = 0; |
| 83 | } |
| 84 | |
| 85 | @@ -466,7 +474,7 @@ static int get_next_block(bunzip_data *bd) |
| 86 | first symbol in the mtf array, position 0, would have been handled |
| 87 | as part of a run above. Therefore 1 unused mtf position minus |
| 88 | 2 non-literal nextSym values equals -1.) */ |
| 89 | - if (dbufCount >= dbufSize) return RETVAL_DATA_ERROR; |
| 90 | + if (dbufCount >= bd->dbufSize) return RETVAL_DATA_ERROR; |
| 91 | i = nextSym - 1; |
| 92 | uc = mtfSymbol[i]; |
| 93 | |
| 94 | -- |
| 95 | cgit v0.12 |