Merge branch 'maint-0.3.1'

changes/bug22669

@@ -0,0 +1,4 @@
+  o Minor bugfixes (compression):
+    - When serving directory votes compressed with zlib,
+      do not claim to have compressed them with zstd. Fixes bug 22669;
+      bugfix on 0.3.1.1-alpha.

changes/bug22672

@@ -0,0 +1,5 @@
+  o Minor features (compression, defensive programming):
+    - Detect and break out of infinite loops in our compression code.
+      We don't think that any such loops exist now, but it's best to be
+      safe. Closes ticket 22672.
+

src/common/compress.c

@@ -150,13 +150,7 @@ tor_compress_impl(int compress,
                     method, compression_level, (unsigned long)in_len);
           goto err;
         } else {
-          if (in_len != 0) {
-            log_fn(protocol_warn_level, LD_PROTOCOL,
-                   "Unexpected extra input while decompressing");
-            log_debug(LD_GENERAL, "method: %d level: %d at len: %lu",
-                      method, compression_level, (unsigned long)in_len);
-            goto err;
-          } else {
+          if (in_len == 0) {
             goto done;
           }
         }
@@ -548,28 +542,42 @@ tor_compress_process(tor_compress_state_t *state,
                      int finish)
 {
   tor_assert(state != NULL);
+  const size_t in_len_orig = *in_len;
+  const size_t out_len_orig = *out_len;
+  tor_compress_output_t rv;
 
   switch (state->method) {
     case GZIP_METHOD:
     case ZLIB_METHOD:
-      return tor_zlib_compress_process(state->u.zlib_state,
+      rv = tor_zlib_compress_process(state->u.zlib_state,
                                      out, out_len, in, in_len,
                                      finish);
+      break;
     case LZMA_METHOD:
-      return tor_lzma_compress_process(state->u.lzma_state,
+      rv =tor_lzma_compress_process(state->u.lzma_state,
                                     out, out_len, in, in_len,
                                     finish);
+      break;
     case ZSTD_METHOD:
-      return tor_zstd_compress_process(state->u.zstd_state,
+      rv = tor_zstd_compress_process(state->u.zstd_state,
                                      out, out_len, in, in_len,
                                      finish);
+      break;
     case NO_METHOD:
-      return tor_cnone_compress_process(out, out_len, in, in_len,
+      rv = tor_cnone_compress_process(out, out_len, in, in_len,
                                       finish);
+      break;
+    default:
     case UNKNOWN_METHOD:
       goto err;
   }
+  if (BUG((rv == TOR_COMPRESS_OK) &&
+          *in_len == in_len_orig &&
+          *out_len == out_len_orig)) {
+    return TOR_COMPRESS_ERROR;
+  }
 
+  return rv;
  err:
   return TOR_COMPRESS_ERROR;
 }

src/or/directory.c

@@ -4190,13 +4190,14 @@ static int
 handle_get_status_vote(dir_connection_t *conn, const get_handler_args_t *args)
 {
   const char *url = args->url;
-  const compress_method_t compress_method =
-    find_best_compression_method(args->compression_supported, 1);
   {
     int current;
     ssize_t body_len = 0;
     ssize_t estimated_len = 0;
+    /* This smartlist holds strings that we can compress on the fly. */
     smartlist_t *items = smartlist_new();
+    /* This smartlist holds cached_dir_t objects that have a precompressed
+     * deflated version. */
     smartlist_t *dir_items = smartlist_new();
     int lifetime = 60; /* XXXX?? should actually use vote intervals. */
     url += strlen("/tor/status-vote/");
@@ -4247,6 +4248,26 @@ handle_get_status_vote(dir_connection_t *conn, const get_handler_args_t *args)
       write_http_status_line(conn, 404, "Not found");
       goto vote_done;
     }
+
+    /* We're sending items from at most one kind of source */
+    tor_assert_nonfatal(smartlist_len(items) == 0 ||
+                        smartlist_len(dir_items) == 0);
+
+    int streaming;
+    unsigned mask;
+    if (smartlist_len(items)) {
+      /* We're taking strings and compressing them on the fly. */
+      streaming = 1;
+      mask = ~0u;
+    } else {
+      /* We're taking cached_dir_t objects. We only have them uncompressed
+       * or deflated. */
+      streaming = 0;
+      mask = (1u<<NO_METHOD) | (1u<<ZLIB_METHOD);
+    }
+    const compress_method_t compress_method = find_best_compression_method(
+                              args->compression_supported&mask, streaming);
+
     SMARTLIST_FOREACH(dir_items, cached_dir_t *, d,
                       body_len += compress_method != NO_METHOD ?
                         d->dir_compressed_len : d->dir_len);

src/test/test_util.c

@@ -2398,6 +2398,59 @@ test_util_compress(void *arg)
   ;
 }
 
+static void
+test_util_decompress_concatenated_impl(compress_method_t method)
+{
+  char input[4096];
+  char *c1 = NULL, *c2 = NULL, *c3 = NULL;
+  char *result = NULL;
+  size_t sz1, sz2, sz3, szr;
+  int r;
+
+  crypto_rand(input, sizeof(input));
+
+  /* Compress the input in two chunks. */
+  r = tor_compress(&c1, &sz1, input, 2048, method);
+  tt_int_op(r, OP_EQ, 0);
+  r = tor_compress(&c2, &sz2, input+2048, 2048, method);
+  tt_int_op(r, OP_EQ, 0);
+
+  /* concatenate the chunks. */
+  sz3 = sz1 + sz2;
+  c3 = tor_malloc(sz3);
+  memcpy(c3, c1, sz1);
+  memcpy(c3+sz1, c2, sz2);
+
+  /* decompress the concatenated result */
+  r = tor_uncompress(&result, &szr, c3, sz3, method, 0, LOG_WARN);
+  tt_int_op(r, OP_EQ, 0);
+  tt_int_op(szr, OP_EQ, sizeof(input));
+  tt_mem_op(result, OP_EQ, input, sizeof(input));
+
+ done:
+  tor_free(c1);
+  tor_free(c2);
+  tor_free(c3);
+  tor_free(result);
+}
+
+static void
+test_util_decompress_concatenated(void *arg)
+{
+  const char *methodname = arg;
+  tt_assert(methodname);
+
+  compress_method_t method = compression_method_get_by_name(methodname);
+  tt_int_op(method, OP_NE, UNKNOWN_METHOD);
+  if (! tor_compress_supports_method(method)) {
+    tt_skip();
+  }
+
+  test_util_decompress_concatenated_impl(method);
+ done:
+  ;
+}
+
 static void
 test_util_gzip_compression_bomb(void *arg)
 {
@@ -5819,6 +5872,11 @@ test_util_get_unquoted_path(void *arg)
   { "compress/" #name, test_util_compress, 0, &passthrough_setup,       \
     (char*)(identifier) }
 
+#define COMPRESS_CONCAT(name, identifier)                               \
+  { "compress_concat/" #name, test_util_decompress_concatenated, 0,     \
+    &passthrough_setup,                                                 \
+    (char*)(identifier) }
+
 #ifdef _WIN32
 #define UTIL_TEST_NO_WIN(n, f) { #n, NULL, TT_SKIP, NULL, NULL }
 #define UTIL_TEST_WIN_ONLY(n, f) UTIL_TEST(n, (f))
@@ -5848,6 +5906,11 @@ struct testcase_t util_tests[] = {
   COMPRESS(lzma, "x-tor-lzma"),
   COMPRESS(zstd, "x-zstd"),
   COMPRESS(none, "identity"),
+  COMPRESS_CONCAT(zlib, "deflate"),
+  COMPRESS_CONCAT(gzip, "gzip"),
+  COMPRESS_CONCAT(lzma, "x-tor-lzma"),
+  COMPRESS_CONCAT(zstd, "x-zstd"),
+  COMPRESS_CONCAT(none, "identity"),
   UTIL_TEST(gzip_compression_bomb, TT_FORK),
   UTIL_LEGACY(datadir),
   UTIL_LEGACY(memarea),