[tor-commits] [tor/release-0.2.8] Fix a double-free bug in routerlist_reparse_old

[nickm@xxxxxxxxxxxxxx]

commit 9cf6af76eb250dc0716185e349a92b2def6981d3
Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Date:   Wed May 25 11:52:52 2016 -0400

    Fix a double-free bug in routerlist_reparse_old
    
    I introduced this bug when I moved signing_key_cert into
    signed_descriptor_t. Bug not in any released Tor.  Fixes bug 19175, and
    another case of 19128.
    
    Just like signed_descriptor_from_routerinfo(), routerlist_reparse_old()
    copies the fields from one signed_descriptor_t to another, and then
    clears the fields from the original that would have been double-freed by
    freeing the original.  But when I fixed the s_d_f_r() bug [#19128] in
    50cbf220994c7cec593, I missed the fact that the code was duplicated in
    r_p_o().
    
    Duplicated code strikes again!
    
    For a longer-term solution here, I am not only adding the missing fix to
    r_p_o(): I am also extracting the duplicated code into a new function.
    
    Many thanks to toralf for patiently sending me stack traces until
    one made sense.
---
 src/or/routerlist.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index d49814f..a08b5f3 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -2938,6 +2938,19 @@ signed_descriptor_free(signed_descriptor_t *sd)
   tor_free(sd);
 }
 
+/** Copy src into dest, and steal all references inside src so that when
+ * we free src, we don't mess up dest. */
+static void
+signed_descriptor_move(signed_descriptor_t *dest,
+                       signed_descriptor_t *src)
+{
+  tor_assert(dest != src);
+  memcpy(dest, src, sizeof(signed_descriptor_t));
+  src->signed_descriptor_body = NULL;
+  src->signing_key_cert = NULL;
+  dest->routerlist_index = -1;
+}
+
 /** Extract a signed_descriptor_t from a general routerinfo, and free the
  * routerinfo.
  */
@@ -2947,10 +2960,7 @@ signed_descriptor_from_routerinfo(routerinfo_t *ri)
   signed_descriptor_t *sd;
   tor_assert(ri->purpose == ROUTER_PURPOSE_GENERAL);
   sd = tor_malloc_zero(sizeof(signed_descriptor_t));
-  memcpy(sd, &(ri->cache_info), sizeof(signed_descriptor_t));
-  sd->routerlist_index = -1;
-  ri->cache_info.signed_descriptor_body = NULL;
-  ri->cache_info.signing_key_cert = NULL;
+  signed_descriptor_move(sd, &ri->cache_info);
   routerinfo_free(ri);
   return sd;
 }
@@ -3436,9 +3446,7 @@ routerlist_reparse_old(routerlist_t *rl, signed_descriptor_t *sd)
                          0, 1, NULL, NULL);
   if (!ri)
     return NULL;
-  memcpy(&ri->cache_info, sd, sizeof(signed_descriptor_t));
-  sd->signed_descriptor_body = NULL; /* Steal reference. */
-  ri->cache_info.routerlist_index = -1;
+  signed_descriptor_move(&ri->cache_info, sd);
 
   routerlist_remove_old(rl, sd, -1);
 



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