Merge branch 'bug2324_uncompress' into maint-0.2.1

2 files changed, 63 insertions, 2 deletions

diff --git a/changes/bug2324_uncompress b/changes/bug2324_uncompress
new file mode 100644
index 0000000000..223a3ce35b
--- /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 618b8b0300..249151cc9b 100644
--- a/src/common/torgzip.c
+++ b/src/common/torgzip.c
@@ -57,6 +57,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>.
@@ -159,6 +186,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 +256,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 +296,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 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);
@@ -329,6 +371,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 +442,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: