From f016213f7fde4a0aac359a324baf42ad2356e169 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 8 Jun 2016 18:08:30 -0400 Subject: [PATCH 1/5] Unit tests for our zlib code to test and reject compression bombs. --- changes/test_zlib_bombs | 3 +++ src/test/test_util.c | 48 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 changes/test_zlib_bombs diff --git a/changes/test_zlib_bombs b/changes/test_zlib_bombs new file mode 100644 index 0000000000..26121ed8ce --- /dev/null +++ b/changes/test_zlib_bombs @@ -0,0 +1,3 @@ + o Testing: + - We now have unit tests for our code to reject zlib "compression bombs". + (Fortunately, the code works fine.) diff --git a/src/test/test_util.c b/src/test/test_util.c index c3c6035ba4..8b9d5ea475 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -1836,6 +1836,53 @@ test_util_gzip(void *arg) tor_free(buf1); } +static void +test_util_gzip_compression_bomb(void *arg) +{ + /* A 'compression bomb' is a very small object that uncompresses to a huge + * one. Most compression formats support them, but they can be a DOS vector. + * In Tor we try not to generate them, and we don't accept them. + */ + (void) arg; + size_t one_million = 1<<20; + char *one_mb = tor_malloc_zero(one_million); + char *result = NULL; + size_t result_len = 0; + tor_zlib_state_t *state = NULL; + + /* Make sure we can't produce a compression bomb */ + tt_int_op(-1, OP_EQ, tor_gzip_compress(&result, &result_len, + one_mb, one_million, + ZLIB_METHOD)); + + /* Here's a compression bomb that we made manually. */ + const char compression_bomb[1039] = + { 0x78, 0xDA, 0xED, 0xC1, 0x31, 0x01, 0x00, 0x00, 0x00, 0xC2, + 0xA0, 0xF5, 0x4F, 0x6D, 0x08, 0x5F, 0xA0 /* .... */ }; + tt_int_op(-1, OP_EQ, tor_gzip_uncompress(&result, &result_len, + compression_bomb, 1039, + ZLIB_METHOD, 0, LOG_WARN)); + + + /* Now try streaming that. */ + state = tor_zlib_new(0, ZLIB_METHOD, HIGH_COMPRESSION); + tor_zlib_output_t r; + const char *inp = compression_bomb; + size_t inlen = 1039; + do { + char *outp = one_mb; + size_t outleft = 4096; /* small on purpose */ + r = tor_zlib_process(state, &outp, &outleft, &inp, &inlen, 0); + tt_int_op(inlen, OP_NE, 0); + } while (r == TOR_ZLIB_BUF_FULL); + + tt_int_op(r, OP_EQ, TOR_ZLIB_ERR); + + done: + tor_free(one_mb); + tor_zlib_free(state); +} + /** Run unit tests for mmap() wrapper functionality. */ static void test_util_mmap(void *arg) @@ -4800,6 +4847,7 @@ struct testcase_t util_tests[] = { UTIL_LEGACY(strmisc), UTIL_LEGACY(pow2), UTIL_LEGACY(gzip), + UTIL_LEGACY(gzip_compression_bomb), UTIL_LEGACY(datadir), UTIL_LEGACY(memarea), UTIL_LEGACY(control_formats), From 808015316a5680003b78393c50b0701c47e1051c Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 8 Jun 2016 18:14:51 -0400 Subject: [PATCH 2/5] Remove support for zlib <= 1.1 zlib 1.2 came out in 2003; earlier versions should be dead by now. Our workaround code was only preventing us from using the gzip encoding (if we decide to do so), and having some dead code linger around in torgzip.c --- changes/zlib_12 | 4 ++++ src/common/torgzip.c | 44 ++++---------------------------------------- src/test/test_util.c | 3 ++- 3 files changed, 10 insertions(+), 41 deletions(-) create mode 100644 changes/zlib_12 diff --git a/changes/zlib_12 b/changes/zlib_12 new file mode 100644 index 0000000000..3344286861 --- /dev/null +++ b/changes/zlib_12 @@ -0,0 +1,4 @@ + o New system requirements: + - We now require zlib version 1.2 or later. (Back when we started, + zlib 1.1 and zlib 1.0 were still found in the wild. 1.2 was released in + 2003. We recommend the latest version.) diff --git a/src/common/torgzip.c b/src/common/torgzip.c index 71e55f8723..57994c462e 100644 --- a/src/common/torgzip.c +++ b/src/common/torgzip.c @@ -46,34 +46,16 @@ #include +#if defined ZLIB_VERNUM && ZLIB_VERNUM < 0x1200 +#error "We require zlib version 1.2 or later." +#endif + static size_t tor_zlib_state_size_precalc(int inflate, int windowbits, int memlevel); /** Total number of bytes allocated for zlib state */ static size_t total_zlib_allocation = 0; -/** Set to 1 if zlib is a version that supports gzip; set to 0 if it doesn't; - * set to -1 if we haven't checked yet. */ -static int gzip_is_supported = -1; - -/** Return true iff we support gzip-based compression. Otherwise, we need to - * use zlib. */ -int -is_gzip_supported(void) -{ - if (gzip_is_supported >= 0) - return gzip_is_supported; - - if (!strcmpstart(ZLIB_VERSION, "0.") || - !strcmpstart(ZLIB_VERSION, "1.0") || - !strcmpstart(ZLIB_VERSION, "1.1")) - gzip_is_supported = 0; - else - gzip_is_supported = 1; - - return gzip_is_supported; -} - /** Return a string representation of the version of the currently running * version of zlib. */ const char * @@ -165,12 +147,6 @@ tor_gzip_compress(char **out, size_t *out_len, *out = NULL; - if (method == GZIP_METHOD && !is_gzip_supported()) { - /* Old zlib version don't support gzip in deflateInit2 */ - log_warn(LD_BUG, "Gzip not supported with zlib %s", ZLIB_VERSION); - goto err; - } - stream = tor_malloc_zero(sizeof(struct z_stream_s)); stream->zalloc = Z_NULL; stream->zfree = Z_NULL; @@ -291,12 +267,6 @@ tor_gzip_uncompress(char **out, size_t *out_len, tor_assert(in); tor_assert(in_len < UINT_MAX); - if (method == GZIP_METHOD && !is_gzip_supported()) { - /* Old zlib version don't support gzip in inflateInit2 */ - log_warn(LD_BUG, "Gzip not supported with zlib %s", ZLIB_VERSION); - return -1; - } - *out = NULL; stream = tor_malloc_zero(sizeof(struct z_stream_s)); @@ -451,12 +421,6 @@ tor_zlib_new(int compress, compress_method_t method, tor_zlib_state_t *out; int bits, memlevel; - if (method == GZIP_METHOD && !is_gzip_supported()) { - /* Old zlib version don't support gzip in inflateInit2 */ - log_warn(LD_BUG, "Gzip not supported with zlib %s", ZLIB_VERSION); - return NULL; - } - if (! compress) { /* use this setting for decompression, since we might have the * max number of window bits */ diff --git a/src/test/test_util.c b/src/test/test_util.c index 8b9d5ea475..204d5db25f 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -1737,7 +1737,8 @@ test_util_gzip(void *arg) (void)arg; buf1 = tor_strdup("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAZAAAAAAAAAAAAAAAAAAAZ"); tt_assert(detect_compression_method(buf1, strlen(buf1)) == UNKNOWN_METHOD); - if (is_gzip_supported()) { + + if (1) { tt_assert(!tor_gzip_compress(&buf2, &len1, buf1, strlen(buf1)+1, GZIP_METHOD)); tt_assert(buf2); From 358fc026d97f87166dc3059f56012334317de8ea Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 8 Jun 2016 18:30:23 -0400 Subject: [PATCH 3/5] Remove a ridiculous realloc call from torgzip.c realloc()ing a thing in order to try to save memory on it just doesn't make sense with today's allocators. Instead, let's use the fact that whenever we decompress something, either it isn't too big, or we chop it up, or we reallocate it. --- src/common/torgzip.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/common/torgzip.c b/src/common/torgzip.c index 57994c462e..85114e6bf6 100644 --- a/src/common/torgzip.c +++ b/src/common/torgzip.c @@ -213,10 +213,6 @@ tor_gzip_compress(char **out, size_t *out_len, * the newly unsigned field isn't negative." */ tor_assert(stream->total_out >= 0); #endif - if (((size_t)stream->total_out) > out_size + 4097) { - /* If we're wasting more than 4k, don't. */ - *out = tor_realloc(*out, stream->total_out + 1); - } if (deflateEnd(stream)!=Z_OK) { log_warn(LD_BUG, "Error freeing gzip structures"); goto err; From 5a725dab0a29341019e3ceb041d7ad619df05a76 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 8 Jun 2016 18:34:33 -0400 Subject: [PATCH 4/5] Mark some torgzip lines as unreachable/untestable. --- src/common/torgzip.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/common/torgzip.c b/src/common/torgzip.c index 85114e6bf6..331bb5a017 100644 --- a/src/common/torgzip.c +++ b/src/common/torgzip.c @@ -158,9 +158,11 @@ tor_gzip_compress(char **out, size_t *out_len, method_bits(method, HIGH_COMPRESSION), get_memlevel(HIGH_COMPRESSION), Z_DEFAULT_STRATEGY) != Z_OK) { + //LCOV_EXCL_START -- we can only provoke failure by giving junk arguments. log_warn(LD_GENERAL, "Error from deflateInit2: %s", stream->msg?stream->msg:""); goto err; + //LCOV_EXCL_STOP } /* Guess 50% compression. */ @@ -214,8 +216,11 @@ tor_gzip_compress(char **out, size_t *out_len, tor_assert(stream->total_out >= 0); #endif if (deflateEnd(stream)!=Z_OK) { + // LCOV_EXCL_START -- unreachable if we handled the zlib structure right + tor_assert_nonfatal_unreached(); log_warn(LD_BUG, "Error freeing gzip structures"); goto err; + // LCOV_EXCL_STOP } tor_free(stream); @@ -274,9 +279,11 @@ tor_gzip_uncompress(char **out, size_t *out_len, if (inflateInit2(stream, method_bits(method, HIGH_COMPRESSION)) != Z_OK) { + // LCOV_EXCL_START -- can only hit this if we give bad inputs. log_warn(LD_GENERAL, "Error from inflateInit2: %s", stream->msg?stream->msg:""); goto err; + // LCOV_EXCL_STOP } out_size = in_len * 2; /* guess 50% compression. */ @@ -434,10 +441,10 @@ tor_zlib_new(int compress, compress_method_t method, if (deflateInit2(&out->stream, Z_BEST_COMPRESSION, Z_DEFLATED, bits, memlevel, Z_DEFAULT_STRATEGY) != Z_OK) - goto err; + goto err; // LCOV_EXCL_LINE } else { if (inflateInit2(&out->stream, bits) != Z_OK) - goto err; + goto err; // LCOV_EXCL_LINE } out->allocation = tor_zlib_state_size_precalc(!compress, bits, memlevel); From d937b866999b18e65df3815d0310d9469b15b914 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 19 Jun 2016 12:19:32 -0400 Subject: [PATCH 5/5] Unindent block --- src/test/test_util.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/test/test_util.c b/src/test/test_util.c index 204d5db25f..64eee8cdbd 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -1738,22 +1738,20 @@ test_util_gzip(void *arg) buf1 = tor_strdup("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAZAAAAAAAAAAAAAAAAAAAZ"); tt_assert(detect_compression_method(buf1, strlen(buf1)) == UNKNOWN_METHOD); - if (1) { - tt_assert(!tor_gzip_compress(&buf2, &len1, buf1, strlen(buf1)+1, - GZIP_METHOD)); - tt_assert(buf2); - tt_assert(len1 < strlen(buf1)); - tt_assert(detect_compression_method(buf2, len1) == GZIP_METHOD); + tt_assert(!tor_gzip_compress(&buf2, &len1, buf1, strlen(buf1)+1, + GZIP_METHOD)); + tt_assert(buf2); + tt_assert(len1 < strlen(buf1)); + tt_assert(detect_compression_method(buf2, len1) == GZIP_METHOD); - tt_assert(!tor_gzip_uncompress(&buf3, &len2, buf2, len1, - GZIP_METHOD, 1, LOG_INFO)); - tt_assert(buf3); - tt_int_op(strlen(buf1) + 1,OP_EQ, len2); - tt_str_op(buf1,OP_EQ, buf3); + tt_assert(!tor_gzip_uncompress(&buf3, &len2, buf2, len1, + GZIP_METHOD, 1, LOG_INFO)); + tt_assert(buf3); + tt_int_op(strlen(buf1) + 1,OP_EQ, len2); + tt_str_op(buf1,OP_EQ, buf3); - tor_free(buf2); - tor_free(buf3); - } + tor_free(buf2); + tor_free(buf3); tt_assert(!tor_gzip_compress(&buf2, &len1, buf1, strlen(buf1)+1, ZLIB_METHOD));