Merge branch 'bug2324_uncompress' into maint-0.2.1

This commit is contained in:
Nick Mathewson 2011-01-15 12:12:34 -05:00
commit 8f11642ceb
2 changed files with 63 additions and 2 deletions

View File

@ -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.

View File

@ -57,6 +57,33 @@ method_bits(compress_method_t method)
return method == GZIP_METHOD ? 15+16 : 15; 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 /** 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 * 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>. * 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); 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; return 0;
err: err:
if (stream) { if (stream) {
@ -223,7 +256,7 @@ tor_gzip_uncompress(char **out, size_t *out_len,
out_size = in_len * 2; /* guess 50% compression. */ out_size = in_len * 2; /* guess 50% compression. */
if (out_size < 1024) out_size = 1024; if (out_size < 1024) out_size = 1024;
if (out_size > UINT_MAX) if (out_size >= SIZE_T_CEILING || out_size > UINT_MAX)
goto err; goto err;
*out = tor_malloc(out_size); *out = tor_malloc(out_size);
@ -263,7 +296,16 @@ tor_gzip_uncompress(char **out, size_t *out_len,
old_size = out_size; old_size = out_size;
out_size *= 2; out_size *= 2;
if (out_size < old_size) { 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; goto err;
} }
*out = tor_realloc(*out, out_size); *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 tor_zlib_state_t {
struct z_stream_s stream; struct z_stream_s stream;
int compress; 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 /** 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); 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 = (char*) state->stream.next_out;
*out_len = state->stream.avail_out; *out_len = state->stream.avail_out;
*in = (const char *) state->stream.next_in; *in = (const char *) state->stream.next_in;
*in_len = state->stream.avail_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) switch (err)
{ {
case Z_STREAM_END: case Z_STREAM_END: