Skip to content

Commit

Permalink
Fix a bug that can crash deflate on some input when using Z_FIXED. (c…
Browse files Browse the repository at this point in the history 
…loudflare#29)

Port of the patch for CVE-2018-25032 from
madler@5c44459

This bug was reported by Danilo Ramos of Eideticom, Inc. It has
lain in wait 13 years before being found! The bug was introduced
in zlib 1.2.2.2, with the addition of the Z_FIXED option. That
option forces the use of fixed Huffman codes. For rare inputs with
a large number of distant matches, the pending buffer into which
the compressed data is written can overwrite the distance symbol
table which it overlays. That results in corrupted output due to
invalid distances, and can result in out-of-bound accesses,
crashing the application.

The fix here combines the distance buffer and literal/length
buffers into a single symbol buffer. Now three bytes of pending
buffer space are opened up for each literal or length/distance
pair consumed, instead of the previous two bytes. This assures
that the pending buffer cannot overwrite the symbol table, since
the maximum fixed code compressed length/distance is 31 bits, and
since there are four bytes of pending space for every three bytes
of symbol space.

Co-authored-by: Mark Adler <[email protected]>
  • Loading branch information
@LloydW93 @madler
LloydW93 and madler authored May 26, 2022
1 parent 959b4ea commit c9479d1
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 94 deletions.
109 changes: 64 additions & 45 deletions deflate.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,21 +179,6 @@ static void bulk_insert_str(deflate_state *s, Pos startpos, uint32_t count) {
}
}

static int _tr_tally_lit(deflate_state *s, uint8_t cc) {
s->d_buf[s->last_lit] = 0;
s->l_buf[s->last_lit++] = cc;
s->dyn_ltree[cc].Freq++;
return (s->last_lit == s->lit_bufsize-1);
}

static int _tr_tally_dist(deflate_state *s, uint16_t dist, uint8_t len) {
s->d_buf[s->last_lit] = dist;
s->l_buf[s->last_lit++] = len;
dist--;
s->dyn_ltree[_length_code[len]+LITERALS+1].Freq++;
s->dyn_dtree[d_code(dist)].Freq++;
return (s->last_lit == s->lit_bufsize-1);
}
/* ===========================================================================
* Initialize the hash table prev[] will be initialized on the fly.
*/
Expand Down Expand Up @@ -228,11 +213,6 @@ int ZEXPORT deflateInit2_(strm, level, method, windowBits, memLevel, strategy,
int wrap = 1;
static const char my_version[] = ZLIB_VERSION;

uint16_t *overlay;
/* We overlay pending_buf and d_buf+l_buf. This works since the average
* output size for (length,distance) codes is <= 24 bits.
*/

if (version == Z_NULL || version[0] != my_version[0] ||
stream_size != sizeof(z_stream)) {
return Z_VERSION_ERROR;
Expand Down Expand Up @@ -295,9 +275,47 @@ int ZEXPORT deflateInit2_(strm, level, method, windowBits, memLevel, strategy,

s->lit_bufsize = 1 << (memLevel + 6); /* 16K elements by default */

overlay = (uint16_t *) ZALLOC(strm, s->lit_bufsize, sizeof(uint16_t)+2);
s->pending_buf = (uint8_t *) overlay;
s->pending_buf_size = (uint64_t)s->lit_bufsize * (sizeof(uint16_t)+2L);
/* We overlay pending_buf and sym_buf. This works since the average size
* for length/distance pairs over any compressed block is assured to be 31
* bits or less.
*
* Analysis: The longest fixed codes are a length code of 8 bits plus 5
* extra bits, for lengths 131 to 257. The longest fixed distance codes are
* 5 bits plus 13 extra bits, for distances 16385 to 32768. The longest
* possible fixed-codes length/distance pair is then 31 bits total.
*
* sym_buf starts one-fourth of the way into pending_buf. So there are
* three bytes in sym_buf for every four bytes in pending_buf. Each symbol
* in sym_buf is three bytes -- two for the distance and one for the
* literal/length. As each symbol is consumed, the pointer to the next
* sym_buf value to read moves forward three bytes. From that symbol, up to
* 31 bits are written to pending_buf. The closest the written pending_buf
* bits gets to the next sym_buf symbol to read is just before the last
* code is written. At that time, 31*(n-2) bits have been written, just
* after 24*(n-2) bits have been consumed from sym_buf. sym_buf starts at
* 8*n bits into pending_buf. (Note that the symbol buffer fills when n-1
* symbols are written.) The closest the writing gets to what is unread is
* then n+14 bits. Here n is lit_bufsize, which is 16384 by default, and
* can range from 128 to 32768.
*
* Therefore, at a minimum, there are 142 bits of space between what is
* written and what is read in the overlain buffers, so the symbols cannot
* be overwritten by the compressed data. That space is actually 139 bits,
* due to the three-bit fixed-code block header.
*
* That covers the case where either Z_FIXED is specified, forcing fixed
* codes, or when the use of fixed codes is chosen, because that choice
* results in a smaller compressed block than dynamic codes. That latter
* condition then assures that the above analysis also covers all dynamic
* blocks. A dynamic-code block will only be chosen to be emitted if it has
* fewer bits than a fixed-code block would for the same set of symbols.
* Therefore its average symbol length is assured to be less than 31. So
* the compressed data for a dynamic block also cannot overwrite the
* symbols from which it is being constructed.
*/

s->pending_buf = (uint8_t *) ZALLOC(strm, s->lit_bufsize, 4);
s->pending_buf_size = (uint64_t)s->lit_bufsize * 4;

if (s->window == Z_NULL || s->prev == Z_NULL || s->head == Z_NULL ||
s->pending_buf == Z_NULL) {
Expand All @@ -306,8 +324,12 @@ int ZEXPORT deflateInit2_(strm, level, method, windowBits, memLevel, strategy,
deflateEnd (strm);
return Z_MEM_ERROR;
}
s->d_buf = overlay + s->lit_bufsize/sizeof(uint16_t);
s->l_buf = s->pending_buf + (1+sizeof(uint16_t))*s->lit_bufsize;
s->sym_buf = s->pending_buf + s->lit_bufsize;
s->sym_end = (s->lit_bufsize - 1) * 3;
/* We avoid equality with lit_bufsize*3 because of wraparound at 64K
* on 16 bit machines and because stored blocks are restricted to
* 64K-1 bytes.
*/

s->level = level;
s->strategy = strategy;
Expand Down Expand Up @@ -480,7 +502,7 @@ int ZEXPORT deflatePrime (strm, bits, value)

if (deflateStateCheck(strm)) return Z_STREAM_ERROR;
s = strm->state;
if ((uint8_t *)(s->d_buf) < s->pending_out + ((Buf_size + 7) >> 3))
if ((uint8_t *)(s->sym_buf) < s->pending_out + ((Buf_size + 7) >> 3))
return Z_BUF_ERROR;
do {
put = Buf_size - s->bi_valid;
Expand Down Expand Up @@ -1006,7 +1028,6 @@ int ZEXPORT deflateCopy (dest, source)
{
deflate_state *ds;
deflate_state *ss;
uint16_t *overlay;


if (deflateStateCheck(source) || dest == Z_NULL) {
Expand All @@ -1026,8 +1047,7 @@ int ZEXPORT deflateCopy (dest, source)
ds->window = (uint8_t *) ZALLOC(dest, ds->w_size, 2*sizeof(uint8_t));
ds->prev = (Pos *) ZALLOC(dest, ds->w_size, sizeof(Pos));
ds->head = (Pos *) ZALLOC(dest, ds->hash_size, sizeof(Pos));
overlay = (uint16_t *) ZALLOC(dest, ds->lit_bufsize, sizeof(uint16_t)+2);
ds->pending_buf = (uint8_t *) overlay;
ds->pending_buf = (uint8_t *) ZALLOC(dest, ds->lit_bufsize, sizeof(uint16_t)+2);

if (ds->window == Z_NULL || ds->prev == Z_NULL || ds->head == Z_NULL ||
ds->pending_buf == Z_NULL) {
Expand All @@ -1041,8 +1061,7 @@ int ZEXPORT deflateCopy (dest, source)
zmemcpy(ds->pending_buf, ss->pending_buf, (uint32_t)ds->pending_buf_size);

ds->pending_out = ds->pending_buf + (ss->pending_out - ss->pending_buf);
ds->d_buf = overlay + ds->lit_bufsize/sizeof(uint16_t);
ds->l_buf = ds->pending_buf + (1+sizeof(uint16_t))*ds->lit_bufsize;
ds->sym_buf = ds->pending_buf + ds->lit_bufsize;

ds->l_desc.dyn_tree = ds->dyn_ltree;
ds->d_desc.dyn_tree = ds->dyn_dtree;
Expand Down Expand Up @@ -1610,8 +1629,8 @@ static block_state deflate_fast(s, flush)
if (s->match_length >= ACTUAL_MIN_MATCH) {
check_match(s, s->strstart, s->match_start, s->match_length);

bflush = _tr_tally_dist(s, s->strstart - s->match_start,
s->match_length - MIN_MATCH);
_tr_tally_dist(s, s->strstart - s->match_start,
s->match_length - MIN_MATCH, bflush);

s->lookahead -= s->match_length;

Expand Down Expand Up @@ -1639,7 +1658,7 @@ static block_state deflate_fast(s, flush)
} else {
/* No match, output a literal byte */
Tracevv((stderr,"%c", s->window[s->strstart]));
bflush = _tr_tally_lit (s, s->window[s->strstart]);
_tr_tally_lit (s, s->window[s->strstart], bflush);
s->lookahead--;
s->strstart++;
}
Expand All @@ -1650,7 +1669,7 @@ static block_state deflate_fast(s, flush)
FLUSH_BLOCK(s, 1);
return finish_done;
}
if (s->last_lit)
if (s->sym_next)
FLUSH_BLOCK(s, 0);
return block_done;
}
Expand Down Expand Up @@ -1724,8 +1743,8 @@ static block_state deflate_slow(s, flush)

check_match(s, s->strstart-1, s->prev_match, s->prev_length);

bflush = _tr_tally_dist(s, s->strstart -1 - s->prev_match,
s->prev_length - MIN_MATCH);
_tr_tally_dist(s, s->strstart -1 - s->prev_match,
s->prev_length - MIN_MATCH, bflush);

/* Insert in hash table all strings up to the end of the match.
* strstart-1 and strstart are already inserted. If there is not
Expand Down Expand Up @@ -1753,7 +1772,7 @@ static block_state deflate_slow(s, flush)
* is longer, truncate the previous match to a single literal.
*/
Tracevv((stderr,"%c", s->window[s->strstart-1]));
bflush = _tr_tally_lit(s, s->window[s->strstart-1]);
_tr_tally_lit(s, s->window[s->strstart-1], bflush);
if (bflush) {
FLUSH_BLOCK_ONLY(s, 0);
}
Expand All @@ -1772,15 +1791,15 @@ static block_state deflate_slow(s, flush)
Assert (flush != Z_NO_FLUSH, "no flush?");
if (s->match_available) {
Tracevv((stderr,"%c", s->window[s->strstart-1]));
bflush = _tr_tally_lit(s, s->window[s->strstart-1]);
_tr_tally_lit(s, s->window[s->strstart-1], bflush);
s->match_available = 0;
}
s->insert = s->strstart < ACTUAL_MIN_MATCH-1 ? s->strstart : ACTUAL_MIN_MATCH-1;
if (flush == Z_FINISH) {
FLUSH_BLOCK(s, 1);
return finish_done;
}
if (s->last_lit)
if (s->sym_next)
FLUSH_BLOCK(s, 0);
return block_done;
}
Expand Down Expand Up @@ -1835,15 +1854,15 @@ static block_state deflate_rle(s, flush)
if (s->match_length >= ACTUAL_MIN_MATCH) {
check_match(s, s->strstart, s->strstart - 1, s->match_length);

bflush = _tr_tally_dist(s, 1, s->match_length - MIN_MATCH);
_tr_tally_dist(s, 1, s->match_length - MIN_MATCH, bflush);

s->lookahead -= s->match_length;
s->strstart += s->match_length;
s->match_length = 0;
} else {
/* No match, output a literal byte */
Tracevv((stderr,"%c", s->window[s->strstart]));
bflush = _tr_tally_lit (s, s->window[s->strstart]);
_tr_tally_lit (s, s->window[s->strstart], bflush);
s->lookahead--;
s->strstart++;
}
Expand All @@ -1854,7 +1873,7 @@ static block_state deflate_rle(s, flush)
FLUSH_BLOCK(s, 1);
return finish_done;
}
if (s->last_lit)
if (s->sym_next)
FLUSH_BLOCK(s, 0);
return block_done;
}
Expand Down Expand Up @@ -1883,7 +1902,7 @@ static block_state deflate_huff(s, flush)
/* Output a literal byte */
s->match_length = 0;
Tracevv((stderr,"%c", s->window[s->strstart]));
bflush = _tr_tally_lit (s, s->window[s->strstart]);
_tr_tally_lit (s, s->window[s->strstart], bflush);
s->lookahead--;
s->strstart++;
if (bflush) FLUSH_BLOCK(s, 0);
Expand All @@ -1893,7 +1912,7 @@ static block_state deflate_huff(s, flush)
FLUSH_BLOCK(s, 1);
return finish_done;
}
if (s->last_lit)
if (s->sym_next)
FLUSH_BLOCK(s, 0);
return block_done;
}
34 changes: 23 additions & 11 deletions deflate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ typedef struct internal_state {
/* Depth of each subtree used as tie breaker for trees of equal frequency
*/

uint8_t *l_buf; /* buffer for literals or lengths */
uint8_t *sym_buf; /* buffer for distances and literals/lengths */

uint32_t lit_bufsize;
/* Size of match buffer for literals/lengths. There are 4 reasons for
Expand All @@ -226,13 +226,8 @@ typedef struct internal_state {
* - I can't count above 4
*/

uint32_t last_lit; /* running index in l_buf */

uint16_t *d_buf;
/* Buffer for distances. To simplify the code, d_buf and l_buf have
* the same number of elements. To use different lengths, an extra flag
* array would be necessary.
*/
uInt sym_next; /* running index in sym_buf */
uInt sym_end; /* symbol table full when sym_next reaches this */

uint64_t opt_len; /* bit length of current block with optimal trees */
uint64_t static_len; /* bit length of current block with static trees */
Expand Down Expand Up @@ -311,6 +306,26 @@ void ZLIB_INTERNAL _tr_stored_block(deflate_state *s, uint8_t *buf,
extern const uint8_t ZLIB_INTERNAL _length_code[];
extern const uint8_t ZLIB_INTERNAL _dist_code[];

# define _tr_tally_lit(s, c, flush) \
{ uch cc = (c); \
s->sym_buf[s->sym_next++] = 0; \
s->sym_buf[s->sym_next++] = 0; \
s->sym_buf[s->sym_next++] = cc; \
s->dyn_ltree[cc].Freq++; \
flush = (s->sym_next == s->sym_end); \
}
# define _tr_tally_dist(s, distance, length, flush) \
{ uch len = (uch)(length); \
ush dist = (ush)(distance); \
s->sym_buf[s->sym_next++] = dist; \
s->sym_buf[s->sym_next++] = dist >> 8; \
s->sym_buf[s->sym_next++] = len; \
dist--; \
s->dyn_ltree[_length_code[len]+LITERALS+1].Freq++; \
s->dyn_dtree[d_code(dist)].Freq++; \
flush = (s->sym_next == s->sym_end); \
}

#ifdef _MSC_VER

/* MSC doesn't have __builtin_expect. Just ignore likely/unlikely and
Expand All @@ -322,9 +337,6 @@ extern const uint8_t ZLIB_INTERNAL _dist_code[];
int __inline __builtin_ctzl(unsigned long mask)
{
unsigned long index ;

return _BitScanForward(&index, mask) == 0 ? 32 : ((int)index) ;
}
#else
#define likely(x) __builtin_expect((x),1)
#define unlikely(x) __builtin_expect((x),0)
Expand Down
Loading

0 comments on commit c9479d1

Please sign in to comment.