[tor-commits] [tor/release-0.2.6] Repair breakage in early-error case of microdesc parsing

[nickm at torproject.org]

commit e0b7598833238766b157f8eb799f448dac4c1283
Author: Nick Mathewson <nickm at torproject.org>
Date:   Mon Jun 22 13:51:56 2015 -0400

    Repair breakage in early-error case of microdesc parsing
    
    When I fixed #11243, I made it so we would take the digest of a
    descriptor before tokenizing it, so we could desist from download
    attempts if parsing failed.  But when I did that, I didn't remove an
    assertion that the descriptor began with "onion-key".  Usually, this
    was enforced by "find_start_of_next_microdescriptor", but when
    find_start_of_next_microdescriptor returned NULL, the assertion was
    triggered.
    
    Fixes bug 16400.  Thanks to torkeln for reporting and
    cypherpunks_backup for diagnosing and writing the first fix here.
---
 changes/bug16400          |    5 +++++
 src/or/routerparse.c      |   14 +++++++++++---
 src/test/test_microdesc.c |   34 ++++++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/changes/bug16400 b/changes/bug16400
new file mode 100644
index 0000000..3e5f9c5
--- /dev/null
+++ b/changes/bug16400
@@ -0,0 +1,5 @@
+  o Major bugfixes:
+   - Do not crash with an assertion error when parsing certain kinds
+     of malformed or truncated microdescriptors. Fixes bug 16400;
+     bugfix on 0.2.6.1-alpha. Found by "torkeln"; fix based on a patch by
+     "cypherpunks_backup".
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index 9c66512..dcf4197 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2001 Matej Pfajfar.
+ /* Copyright (c) 2001 Matej Pfajfar.
  * Copyright (c) 2001-2004, Roger Dingledine.
  * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson.
  * Copyright (c) 2007-2015, The Tor Project, Inc. */
@@ -4165,7 +4165,10 @@ microdescs_parse_from_string(const char *s, const char *eos,
     {
       const char *cp = tor_memstr(s, start_of_next_microdesc-s,
                                   "onion-key");
-      tor_assert(cp);
+      const int no_onion_key = (cp == NULL);
+      if (no_onion_key) {
+        cp = s; /* So that we have *some* junk to put in the body */
+      }
 
       md->bodylen = start_of_next_microdesc - cp;
       md->saved_location = where;
@@ -4174,8 +4177,13 @@ microdescs_parse_from_string(const char *s, const char *eos,
       else
         md->body = (char*)cp;
       md->off = cp - start;
+      crypto_digest256(md->digest, md->body, md->bodylen, DIGEST_SHA256);
+      if (no_onion_key) {
+        log_fn(LOG_PROTOCOL_WARN, LD_DIR, "Malformed or truncated descriptor");
+        goto next;
+      }
     }
-    crypto_digest256(md->digest, md->body, md->bodylen, DIGEST_SHA256);
+
 
     if (tokenize_string(area, s, start_of_next_microdesc, tokens,
                         microdesc_token_table, flags)) {
diff --git a/src/test/test_microdesc.c b/src/test/test_microdesc.c
index fb3df77..c037634 100644
--- a/src/test/test_microdesc.c
+++ b/src/test/test_microdesc.c
@@ -713,12 +713,46 @@ test_md_reject_cache(void *arg)
   tor_free(mock_ns_val);
 }
 
+static void
+test_md_corrupt_desc(void *arg)
+{
+  char *cp = NULL;
+  smartlist_t *sl = NULL;
+  (void) arg;
+
+  sl = microdescs_add_to_cache(get_microdesc_cache(),
+                               "@last-listed 2015-06-22 10:00:00\n"
+                               "onion-k\n",
+                               NULL, SAVED_IN_JOURNAL, 0, time(NULL), NULL);
+  tt_int_op(smartlist_len(sl), ==, 0);
+  smartlist_free(sl);
+
+  sl = microdescs_add_to_cache(get_microdesc_cache(),
+                               "@last-listed 2015-06-22 10:00:00\n"
+                               "wiggly\n",
+                               NULL, SAVED_IN_JOURNAL, 0, time(NULL), NULL);
+  tt_int_op(smartlist_len(sl), ==, 0);
+  smartlist_free(sl);
+
+  tor_asprintf(&cp, "%s\n%s", test_md1, "@foobar\nonion-wobble\n");
+
+  sl = microdescs_add_to_cache(get_microdesc_cache(),
+                               cp, cp+strlen(cp),
+                               SAVED_IN_JOURNAL, 0, time(NULL), NULL);
+  tt_int_op(smartlist_len(sl), ==, 0);
+  smartlist_free(sl);
+
+ done:
+  tor_free(cp);
+}
+
 struct testcase_t microdesc_tests[] = {
   { "cache", test_md_cache, TT_FORK, NULL, NULL },
   { "broken_cache", test_md_cache_broken, TT_FORK, NULL, NULL },
   { "generate", test_md_generate, 0, NULL, NULL },
   { "parse", test_md_parse, 0, NULL, NULL },
   { "reject_cache", test_md_reject_cache, TT_FORK, NULL, NULL },
+  { "corrupt_desc", test_md_corrupt_desc, TT_FORK, NULL, NULL },
   END_OF_TESTCASES
 };