Skip to content

Commit

Permalink
Fixed two issues found with fuzzing.
Browse files Browse the repository at this point in the history 
The first fix tackles a heap overread caused by the prev_length being set to 0 under certain circumstances in deflate.c.

The second issue is a 1-byte heap overflow caused by an incorrect computation of the max_block_size used in emitting stored blocks. Under certain circumstances, bi_window puts a short with (2 byte) causing the header to be 6 bytes instead of the assumed 5 and max_block_size to be 506 instead of 507.
  • Loading branch information
@mschwarzl @kornelski
mschwarzl authored and kornelski committed Nov 16, 2023
1 parent 886098f commit 8352d10
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions deflate.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1137,7 +1137,7 @@ static uint32_t longest_match(deflate_state *s, IPos cur_match /* current match
register uint8_t *scan = s->window + s->strstart; /* current string */
register uint8_t *match; /* matched string */
register int len; /* length of current match */
int best_len = s->prev_length; /* best match length so far */
int best_len = (s->prev_length == 0) ? ACTUAL_MIN_MATCH-1 : s->prev_length; /* best match length so far */
int nice_match = s->nice_match; /* stop if match long enough */
IPos limit = s->strstart > (IPos)MAX_DIST(s) ?
s->strstart - (IPos)MAX_DIST(s) : NIL;
Expand Down Expand Up @@ -1467,8 +1467,8 @@ static block_state deflate_stored(deflate_state *s, int flush)
uint64_t max_block_size = 0xffff;
uint64_t max_start;

if (max_block_size > s->pending_buf_size - 5) {
max_block_size = s->pending_buf_size - 5;
if (max_block_size > s->pending_buf_size - 6) {
max_block_size = s->pending_buf_size - 6;
}

/* Copy as much as possible from input to output: */
Expand Down Expand Up @@ -1688,7 +1688,7 @@ static block_state deflate_slow(deflate_state *s, int flush) {
insert_cnt = max_insert - s->strstart;

bulk_insert_str(s, s->strstart + 1, insert_cnt);
s->prev_length = 0;
s->prev_length = ACTUAL_MIN_MATCH-1;

This comment has been minimized.

Sign in to view

Copy link
@pracj3am

pracj3am Nov 21, 2023

This change very likely causes segfault:

Program received signal SIGSEGV, Segmentation fault.
0x00007f97b289d8bc in _tr_flush_bits () from /usr/local/lib/libz.so.1
(gdb) bt
#0  0x00007f97b289d8bc in _tr_flush_bits () from /usr/local/lib/libz.so.1
#1  0x00007f97b289288d in deflate_slow () from /usr/local/lib/libz.so.1
#2  0x00007f97b2893878 in deflate () from /usr/local/lib/libz.so.1
#3  0x00007f97b289efa6 in compress2 () from /usr/local/lib/libz.so.1
#4  0x00007f97b2ad46a2 in ?? () from /lib/x86_64-linux-gnu/libbfd-2.40-system.so

when calling

objcopy --only-keep-debug --compress-debug-sections debian/nginx/usr/sbin/nginx debian/.debhelper/nginx/dbgsym-root/usr/lib/debug/.build-id/f2/40c5907d85fee13a723bdf9cce9a5edef31673.debug
s->match_available = 0;
s->match_length = ACTUAL_MIN_MATCH-1;
s->strstart += mov_fwd + 1;
Expand Down

1 comment on commit 8352d10

@mschwarzl
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

just reproduced your issue using configure and make and it is not caused by my changes.
The problem is a missing definition of HAS_SSE2.
When compiled with cmake the defines are correct. However, when doing that with configure and make the definition of HAS_SSE2 is not set.

This change in commit 5666c2d
causes the problem:

#elif defined __x86_64__ || defined _M_AMD64
#elif defined HAS_SSE2

This PR fixes the issue in configure:
#54

Please sign in to comment.