commit e0b7598833238766b157f8eb799f448dac4c1283 Author: Nick Mathewson <nickm@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 };