[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[or-cvs] [tor/master] Detect and disallow compression bombs
commit 64798dab4f4fa9404c92d98cdb10d312b1f6e556
Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Date: Mon Jan 3 15:54:23 2011 -0500
Detect and disallow compression bombs
---
changes/bug2324_uncompress | 5 ++++
src/common/torgzip.c | 51 ++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 54 insertions(+), 2 deletions(-)
diff --git a/changes/bug2324_uncompress b/changes/bug2324_uncompress
new file mode 100644
index 0000000..223a3ce
--- /dev/null
+++ b/changes/bug2324_uncompress
@@ -0,0 +1,5 @@
+ o Major bugfixes (security):
+ - Prevent a DoS attack by disallowing any zlib-compressed data
+ whose compression factor is implausibly high. Fixes the
+ second part of bug2324; found by doors.
+
diff --git a/src/common/torgzip.c b/src/common/torgzip.c
index 618b8b0..b44b159 100644
--- a/src/common/torgzip.c
+++ b/src/common/torgzip.c
@@ -57,6 +57,24 @@ method_bits(compress_method_t method)
return method == GZIP_METHOD ? 15+16 : 15;
}
+/* These macros define the maximum allowable compression factor. Anything of
+ * size greater than <b>check_for_compression_bomb_after</b> is not allowed to
+ * have an uncompression factor (uncompressed size:compressed size ratio) of
+ * any greater than MAX_UNCOMPRESSION_FACTOR. */
+#define MAX_UNCOMPRESSION_FACTOR 25
+#define CHECK_FOR_COMPRESSION_BOMB_AFTER (1024*64)
+
+/** Return true if uncompressing an input of size <b>in_size</b> to an input
+ * of size at least <b>size_out</b> looks like a compression bomb. */
+static int
+is_compression_bomb(size_t size_in, size_t size_out)
+{
+ if (size_in == 0 || size_out < CHECK_FOR_COMPRESSION_BOMB_AFTER)
+ return 0;
+
+ return (size_out / size_in > MAX_UNCOMPRESSION_FACTOR);
+}
+
/** Given <b>in_len</b> bytes at <b>in</b>, compress them into a newly
* allocated buffer, using the method described in <b>method</b>. Store the
* compressed string in *<b>out</b>, and its length in *<b>out_len</b>.
@@ -159,6 +177,12 @@ tor_gzip_compress(char **out, size_t *out_len,
}
tor_free(stream);
+ if (is_compression_bomb(*out_len, in_len)) {
+ log_warn(LD_BUG, "We compressed something and got an insanely high "
+ "compression factor; other Tors would think this was a zlib bomb.");
+ goto err;
+ }
+
return 0;
err:
if (stream) {
@@ -223,7 +247,7 @@ tor_gzip_uncompress(char **out, size_t *out_len,
out_size = in_len * 2; /* guess 50% compression. */
if (out_size < 1024) out_size = 1024;
- if (out_size > UINT_MAX)
+ if (out_size > SIZE_T_CEILING || out_size > UINT_MAX)
goto err;
*out = tor_malloc(out_size);
@@ -263,7 +287,16 @@ tor_gzip_uncompress(char **out, size_t *out_len,
old_size = out_size;
out_size *= 2;
if (out_size < old_size) {
- log_warn(LD_GENERAL, "Size overflow in compression.");
+ log_warn(LD_GENERAL, "Size overflow in uncompression.");
+ goto err;
+ }
+ if (is_compression_bomb(in_len, out_size)) {
+ log_warn(LD_GENERAL, "Input looks look a possible zlib bomb; "
+ "not proceeding.");
+ goto err;
+ }
+ if (out_size >= SIZE_T_CEILING) {
+ log_warn(LD_BUG, "Hit SIZE_T_CEILING limit while uncompressing.");
goto err;
}
*out = tor_realloc(*out, out_size);
@@ -329,6 +362,11 @@ detect_compression_method(const char *in, size_t in_len)
struct tor_zlib_state_t {
struct z_stream_s stream;
int compress;
+
+ /* Number of bytes read so far. Used to detect zlib bombs. */
+ size_t input_so_far;
+ /* Number of bytes written so far. Used to detect zlib bombs. */
+ size_t output_so_far;
};
/** Construct and return a tor_zlib_state_t object using <b>method</b>. If
@@ -395,11 +433,20 @@ tor_zlib_process(tor_zlib_state_t *state,
err = inflate(&state->stream, finish ? Z_FINISH : Z_SYNC_FLUSH);
}
+ state->input_so_far += state->stream.next_in - ((unsigned char*)*in);
+ state->output_so_far += state->stream.next_out - ((unsigned char*)*out);
+
*out = (char*) state->stream.next_out;
*out_len = state->stream.avail_out;
*in = (const char *) state->stream.next_in;
*in_len = state->stream.avail_in;
+ if (! state->compress &&
+ is_compression_bomb(state->input_so_far, state->output_so_far)) {
+ log_warn(LD_DIR, "Possible zlib bomb; abandoning stream.");
+ return TOR_ZLIB_ERR;
+ }
+
switch (err)
{
case Z_STREAM_END:
From d14b0d54d2469744266769d7e61135d1aa1c9c11 Mon Sep 17 00:00:00 2001
Patch-Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Subject: [tor/master] Fix a SIZE_T_CEILING check in torgzip.c; noticed by cypherpunks
commit d14b0d54d2469744266769d7e61135d1aa1c9c11
Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Date: Wed Jan 5 12:42:34 2011 -0500
Fix a SIZE_T_CEILING check in torgzip.c; noticed by cypherpunks
---
src/common/torgzip.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/common/torgzip.c b/src/common/torgzip.c
index b44b159..7678668 100644
--- a/src/common/torgzip.c
+++ b/src/common/torgzip.c
@@ -247,7 +247,7 @@ tor_gzip_uncompress(char **out, size_t *out_len,
out_size = in_len * 2; /* guess 50% compression. */
if (out_size < 1024) out_size = 1024;
- if (out_size > SIZE_T_CEILING || out_size > UINT_MAX)
+ if (out_size >= SIZE_T_CEILING || out_size > UINT_MAX)
goto err;
*out = tor_malloc(out_size);
From 1fcfc186284a375bab2595162564f0dd6c1d19f0 Mon Sep 17 00:00:00 2001
Patch-Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Subject: [tor/master] clean up message; explain a magic number in a comment
commit 1fcfc186284a375bab2595162564f0dd6c1d19f0
Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Date: Sat Jan 15 12:12:10 2011 -0500
clean up message; explain a magic number in a comment
---
src/common/torgzip.c | 15 ++++++++++++---
1 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/common/torgzip.c b/src/common/torgzip.c
index 7678668..249151c 100644
--- a/src/common/torgzip.c
+++ b/src/common/torgzip.c
@@ -58,9 +58,18 @@ method_bits(compress_method_t method)
}
/* These macros define the maximum allowable compression factor. Anything of
- * size greater than <b>check_for_compression_bomb_after</b> is not allowed to
+ * size greater than CHECK_FOR_COMPRESSION_BOMB_AFTER is not allowed to
* have an uncompression factor (uncompressed size:compressed size ratio) of
- * any greater than MAX_UNCOMPRESSION_FACTOR. */
+ * any greater than MAX_UNCOMPRESSION_FACTOR.
+ *
+ * Picking a value for MAX_UNCOMPRESSION_FACTOR is a trade-off: we want it to
+ * be small to limit the attack multiplier, but we also want it to be large
+ * enough so that no legitimate document --even ones we might invent in the
+ * future -- ever compresses by a factor of greater than
+ * MAX_UNCOMPRESSION_FACTOR. Within those parameters, there's a reasonably
+ * large range of possible values. IMO, anything over 8 is probably safe; IMO
+ * anything under 50 is probably sufficient.
+ */
#define MAX_UNCOMPRESSION_FACTOR 25
#define CHECK_FOR_COMPRESSION_BOMB_AFTER (1024*64)
@@ -291,7 +300,7 @@ tor_gzip_uncompress(char **out, size_t *out_len,
goto err;
}
if (is_compression_bomb(in_len, out_size)) {
- log_warn(LD_GENERAL, "Input looks look a possible zlib bomb; "
+ log_warn(LD_GENERAL, "Input looks like a possible zlib bomb; "
"not proceeding.");
goto err;
}
From 8f11642ceb357c7ff057335f14c37c3f7b33644f Mon Sep 17 00:00:00 2001
Patch-Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Subject: [tor/master] Merge branch 'bug2324_uncompress' into maint-0.2.1
commit 8f11642ceb357c7ff057335f14c37c3f7b33644f
Merge: 50b06a2 1fcfc18
Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Date: Sat Jan 15 12:12:34 2011 -0500
Merge branch 'bug2324_uncompress' into maint-0.2.1
changes/bug2324_uncompress | 5 +++
src/common/torgzip.c | 60 ++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 63 insertions(+), 2 deletions(-)
From cff4cfef4ff8813fbc47a9f914bb09bc011cb101 Mon Sep 17 00:00:00 2001
Patch-Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Subject: [tor/master] Merge remote branch 'origin/maint-0.2.1' into maint-0.2.2
commit cff4cfef4ff8813fbc47a9f914bb09bc011cb101
Merge: ed87738 8f11642
Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Date: Sat Jan 15 12:13:50 2011 -0500
Merge remote branch 'origin/maint-0.2.1' into maint-0.2.2
changes/bug2324_uncompress | 5 +++
src/common/torgzip.c | 60 ++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 63 insertions(+), 2 deletions(-)
diff --combined src/common/torgzip.c
index 8c4dca9,249151c..b2a205f
--- a/src/common/torgzip.c
+++ b/src/common/torgzip.c
@@@ -13,42 -13,20 +13,42 @@@
#include <stdlib.h>
#include <stdio.h>
#include <assert.h>
-#ifdef _MSC_VER
-#include "..\..\contrib\zlib\zlib.h"
-#else
-#include <zlib.h>
-#endif
#include <string.h>
#ifdef HAVE_NETINET_IN_H
#include <netinet/in.h>
#endif
+#include "torint.h"
#include "util.h"
-#include "log.h"
+#include "torlog.h"
#include "torgzip.h"
+/* zlib 1.2.4 and 1.2.5 do some "clever" things with macros. Instead of
+ saying "(defined(FOO) ? FOO : 0)" they like to say "FOO-0", on the theory
+ that nobody will care if the compile outputs a no-such-identifier warning.
+
+ Sorry, but we like -Werror over here, so I guess we need to define these.
+ I hope that zlib 1.2.6 doesn't break these too.
+*/
+#ifndef _LARGEFILE64_SOURCE
+#define _LARGEFILE64_SOURCE 0
+#endif
+#ifndef _LFS64_LARGEFILE
+#define _LFS64_LARGEFILE 0
+#endif
+#ifndef _FILE_OFFSET_BITS
+#define _FILE_OFFSET_BITS 0
+#endif
+#ifndef off64_t
+#define off64_t int64_t
+#endif
+
+#ifdef _MSC_VER
+#include "..\..\contrib\zlib\zlib.h"
+#else
+#include <zlib.h>
+#endif
+
/** 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;
@@@ -79,6 -57,33 +79,33 @@@ method_bits(compress_method_t method
return method == GZIP_METHOD ? 15+16 : 15;
}
+ /* These macros define the maximum allowable compression factor. Anything of
+ * size greater than CHECK_FOR_COMPRESSION_BOMB_AFTER is not allowed to
+ * have an uncompression factor (uncompressed size:compressed size ratio) of
+ * any greater than MAX_UNCOMPRESSION_FACTOR.
+ *
+ * Picking a value for MAX_UNCOMPRESSION_FACTOR is a trade-off: we want it to
+ * be small to limit the attack multiplier, but we also want it to be large
+ * enough so that no legitimate document --even ones we might invent in the
+ * future -- ever compresses by a factor of greater than
+ * MAX_UNCOMPRESSION_FACTOR. Within those parameters, there's a reasonably
+ * large range of possible values. IMO, anything over 8 is probably safe; IMO
+ * anything under 50 is probably sufficient.
+ */
+ #define MAX_UNCOMPRESSION_FACTOR 25
+ #define CHECK_FOR_COMPRESSION_BOMB_AFTER (1024*64)
+
+ /** Return true if uncompressing an input of size <b>in_size</b> to an input
+ * of size at least <b>size_out</b> looks like a compression bomb. */
+ static int
+ is_compression_bomb(size_t size_in, size_t size_out)
+ {
+ if (size_in == 0 || size_out < CHECK_FOR_COMPRESSION_BOMB_AFTER)
+ return 0;
+
+ return (size_out / size_in > MAX_UNCOMPRESSION_FACTOR);
+ }
+
/** Given <b>in_len</b> bytes at <b>in</b>, compress them into a newly
* allocated buffer, using the method described in <b>method</b>. Store the
* compressed string in *<b>out</b>, and its length in *<b>out_len</b>.
@@@ -181,13 -186,21 +208,19 @@@ tor_gzip_compress(char **out, size_t *o
}
tor_free(stream);
+ if (is_compression_bomb(*out_len, in_len)) {
+ log_warn(LD_BUG, "We compressed something and got an insanely high "
+ "compression factor; other Tors would think this was a zlib bomb.");
+ goto err;
+ }
+
return 0;
err:
if (stream) {
deflateEnd(stream);
tor_free(stream);
}
- if (*out) {
- tor_free(*out);
- }
+ tor_free(*out);
return -1;
}
@@@ -243,7 -256,7 +276,7 @@@ tor_gzip_uncompress(char **out, size_t
out_size = in_len * 2; /* guess 50% compression. */
if (out_size < 1024) out_size = 1024;
- if (out_size > UINT_MAX)
+ if (out_size >= SIZE_T_CEILING || out_size > UINT_MAX)
goto err;
*out = tor_malloc(out_size);
@@@ -283,7 -296,16 +316,16 @@@
old_size = out_size;
out_size *= 2;
if (out_size < old_size) {
- log_warn(LD_GENERAL, "Size overflow in compression.");
+ log_warn(LD_GENERAL, "Size overflow in uncompression.");
+ goto err;
+ }
+ if (is_compression_bomb(in_len, out_size)) {
+ log_warn(LD_GENERAL, "Input looks like a possible zlib bomb; "
+ "not proceeding.");
+ goto err;
+ }
+ if (out_size >= SIZE_T_CEILING) {
+ log_warn(LD_BUG, "Hit SIZE_T_CEILING limit while uncompressing.");
goto err;
}
*out = tor_realloc(*out, out_size);
@@@ -349,6 -371,11 +391,11 @@@ detect_compression_method(const char *i
struct tor_zlib_state_t {
struct z_stream_s stream;
int compress;
+
+ /* Number of bytes read so far. Used to detect zlib bombs. */
+ size_t input_so_far;
+ /* Number of bytes written so far. Used to detect zlib bombs. */
+ size_t output_so_far;
};
/** Construct and return a tor_zlib_state_t object using <b>method</b>. If
@@@ -415,11 -442,20 +462,20 @@@ tor_zlib_process(tor_zlib_state_t *stat
err = inflate(&state->stream, finish ? Z_FINISH : Z_SYNC_FLUSH);
}
+ state->input_so_far += state->stream.next_in - ((unsigned char*)*in);
+ state->output_so_far += state->stream.next_out - ((unsigned char*)*out);
+
*out = (char*) state->stream.next_out;
*out_len = state->stream.avail_out;
*in = (const char *) state->stream.next_in;
*in_len = state->stream.avail_in;
+ if (! state->compress &&
+ is_compression_bomb(state->input_so_far, state->output_so_far)) {
+ log_warn(LD_DIR, "Possible zlib bomb; abandoning stream.");
+ return TOR_ZLIB_ERR;
+ }
+
switch (err)
{
case Z_STREAM_END:
@@@ -443,8 -479,7 +499,8 @@@
void
tor_zlib_free(tor_zlib_state_t *state)
{
- tor_assert(state);
+ if (!state)
+ return;
if (state->compress)
deflateEnd(&state->stream);
From f550c96ade61c12cb13f71c5f57fdb0cd97802fb Mon Sep 17 00:00:00 2001
Patch-Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Subject: [tor/master] Merge remote branch 'origin/maint-0.2.2'
commit f550c96ade61c12cb13f71c5f57fdb0cd97802fb
Merge: 1b8f2ef cff4cfe
Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Date: Sat Jan 15 12:16:18 2011 -0500
Merge remote branch 'origin/maint-0.2.2'
changes/bug2324_uncompress | 5 +++
src/common/torgzip.c | 60 ++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 63 insertions(+), 2 deletions(-)