[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-commits] [tor/maint-0.2.4] Avoid free()ing from an mmap on corrupted microdesc cache



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



_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits