nihilist/tor
1
0
Fork 0
mirror of https://gitlab.torproject.org/tpo/core/tor.git synced 2024-11-28 06:13:31 +01:00
Code Issues Packages Projects Releases Wiki Activity

Fix a double-free bug in routerlist_reparse_old

Browse Source 
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
50cbf22099, 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.
This commit is contained in:
Nick Mathewson 2016-05-25 11:52:52 -04:00
parent 6d375f17fc
commit 9cf6af76eb
1 changed files with 15 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
src/or/routerlist.c
View File

@ -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);