commit d8cfa2ef4e6d57f6dd4a33e5b3cfb1a2a12fc4be Author: Nick Mathewson <nickm@torproject.org> Date: Mon Dec 16 13:00:15 2013 -0500 Avoid free()ing from an mmap on corrupted microdesc cache The 'body' field of a microdesc_t holds a strdup()'d value if the microdesc's saved_location field is SAVED_IN_JOURNAL or SAVED_NOWHERE, and holds a pointer to the middle of an mmap if the microdesc is SAVED_IN_CACHE. But we weren't setting that field until a while after we parsed the microdescriptor, which left an interval where microdesc_free() would try to free() the middle of the mmap(). This patch also includes a regression test. This is a fix for #10409; bugfix on 0.2.2.6-alpha. --- changes/bug10409 | 3 +++ src/or/dirvote.c | 3 ++- src/or/microdesc.c | 3 +-- src/or/routerparse.c | 13 ++++++++++--- src/or/routerparse.h | 2 +- src/test/test_microdesc.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 62 insertions(+), 7 deletions(-) diff --git a/changes/bug10409 b/changes/bug10409 new file mode 100644 index 0000000..5ef5ae2 --- /dev/null +++ b/changes/bug10409 @@ -0,0 +1,3 @@ + o Minor bugfixes: + - Avoid a crash bug when starting with a corrupted microdescriptor + cache file. Fix for bug 10406; bugfix on 0.2.2.6-alpha. diff --git a/src/or/dirvote.c b/src/or/dirvote.c index 144859a..ab2225c 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -3538,7 +3538,8 @@ dirvote_create_microdescriptor(const routerinfo_t *ri) { smartlist_t *lst = microdescs_parse_from_string(output, - output+strlen(output), 0, 1); + output+strlen(output), 0, + SAVED_NOWHERE); if (smartlist_len(lst) != 1) { log_warn(LD_DIR, "We generated a microdescriptor we couldn't parse."); SMARTLIST_FOREACH(lst, microdesc_t *, md, microdesc_free(md)); diff --git a/src/or/microdesc.c b/src/or/microdesc.c index b4d22c1..6f9134c 100644 --- a/src/or/microdesc.c +++ b/src/or/microdesc.c @@ -149,11 +149,10 @@ microdescs_add_to_cache(microdesc_cache_t *cache, { smartlist_t *descriptors, *added; const int allow_annotations = (where != SAVED_NOWHERE); - const int copy_body = (where != SAVED_IN_CACHE); descriptors = microdescs_parse_from_string(s, eos, allow_annotations, - copy_body); + where); if (listed_at > 0) { SMARTLIST_FOREACH(descriptors, microdesc_t *, md, md->last_listed = listed_at); diff --git a/src/or/routerparse.c b/src/or/routerparse.c index 299d07d..52f57ec 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -4355,12 +4355,17 @@ find_start_of_next_microdesc(const char *s, const char *eos) /** Parse as many microdescriptors as are found from the string starting at * <b>s</b> and ending at <b>eos</b>. If allow_annotations is set, read any - * annotations we recognize and ignore ones we don't. If <b>copy_body</b> is - * true, then strdup the bodies of the microdescriptors. Return all newly + * annotations we recognize and ignore ones we don't. + * + * If <b>saved_location</b> isn't SAVED_IN_CACHE, make a local copy of each + * descriptor in the body field of each microdesc_t. + * + * Return all newly * parsed microdescriptors in a newly allocated smartlist_t. */ smartlist_t * microdescs_parse_from_string(const char *s, const char *eos, - int allow_annotations, int copy_body) + int allow_annotations, + saved_location_t where) { smartlist_t *tokens; smartlist_t *result; @@ -4369,6 +4374,7 @@ microdescs_parse_from_string(const char *s, const char *eos, const char *start = s; const char *start_of_next_microdesc; int flags = allow_annotations ? TS_ANNOTATIONS_OK : 0; + const int copy_body = (where != SAVED_IN_CACHE); directory_token_t *tok; @@ -4398,6 +4404,7 @@ microdescs_parse_from_string(const char *s, const char *eos, tor_assert(cp); md->bodylen = start_of_next_microdesc - cp; + md->saved_location = where; if (copy_body) md->body = tor_strndup(cp, md->bodylen); else diff --git a/src/or/routerparse.h b/src/or/routerparse.h index c6382a7..e357f18 100644 --- a/src/or/routerparse.h +++ b/src/or/routerparse.h @@ -64,7 +64,7 @@ ns_detached_signatures_t *networkstatus_parse_detached_signatures( smartlist_t *microdescs_parse_from_string(const char *s, const char *eos, int allow_annotations, - int copy_body); + saved_location_t where); authority_cert_t *authority_cert_parse_from_string(const char *s, const char **end_of_string); diff --git a/src/test/test_microdesc.c b/src/test/test_microdesc.c index 89c578f..828e334 100644 --- a/src/test/test_microdesc.c +++ b/src/test/test_microdesc.c @@ -226,8 +226,53 @@ test_md_cache(void *data) tor_free(fn); } +static const char truncated_md[] = + "@last-listed 2013-08-08 19:02:59\n" + "onion-key\n" + "-----BEGIN RSA PUBLIC KEY-----\n" + "MIGJAoGBAM91vLFNaM+gGhnRIdz2Cm/Kl7Xz0cOobIdVzhS3cKUJfk867hCuTipS\n" + "NveLBzNopvgXKruAAzEj3cACxk6Q8lv5UWOGCD1UolkgsWSE62RBjap44g+oc9J1\n" + "RI9968xOTZw0VaBQg9giEILNXl0djoikQ+5tQRUvLDDa67gpa5Q1AgMBAAE=\n" + "-----END RSA PUBLIC KEY-----\n" + "family @\n"; + +static void +test_md_cache_broken(void *data) +{ + or_options_t *options; + char *fn=NULL; + microdesc_cache_t *mc = NULL; + + (void)data; + + options = get_options_mutable(); + tt_assert(options); + options->DataDirectory = tor_strdup(get_fname("md_datadir_test2")); + +#ifdef _WIN32 + tt_int_op(0, ==, mkdir(options->DataDirectory)); +#else + tt_int_op(0, ==, mkdir(options->DataDirectory, 0700)); +#endif + + tor_asprintf(&fn, "%s"PATH_SEPARATOR"cached-microdescs", + options->DataDirectory); + + write_str_to_file(fn, truncated_md, 1); + + mc = get_microdesc_cache(); + tt_assert(mc); + + done: + if (options) + tor_free(options->DataDirectory); + tor_free(fn); + microdesc_free_all(); +} + struct testcase_t microdesc_tests[] = { { "cache", test_md_cache, TT_FORK, NULL, NULL }, + { "broken_cache", test_md_cache_broken, TT_FORK, NULL, NULL }, END_OF_TESTCASES };