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.
This commit is contained in:
Nick Mathewson 2013-12-16 13:00:15 -05:00
parent 9e90707602
commit d8cfa2ef4e
6 changed files with 62 additions and 7 deletions

3
changes/bug10409 Normal file
View File

@ -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.

View File

@ -3538,7 +3538,8 @@ dirvote_create_microdescriptor(const routerinfo_t *ri)
{ {
smartlist_t *lst = microdescs_parse_from_string(output, smartlist_t *lst = microdescs_parse_from_string(output,
output+strlen(output), 0, 1); output+strlen(output), 0,
SAVED_NOWHERE);
if (smartlist_len(lst) != 1) { if (smartlist_len(lst) != 1) {
log_warn(LD_DIR, "We generated a microdescriptor we couldn't parse."); log_warn(LD_DIR, "We generated a microdescriptor we couldn't parse.");
SMARTLIST_FOREACH(lst, microdesc_t *, md, microdesc_free(md)); SMARTLIST_FOREACH(lst, microdesc_t *, md, microdesc_free(md));

View File

@ -149,11 +149,10 @@ microdescs_add_to_cache(microdesc_cache_t *cache,
{ {
smartlist_t *descriptors, *added; smartlist_t *descriptors, *added;
const int allow_annotations = (where != SAVED_NOWHERE); const int allow_annotations = (where != SAVED_NOWHERE);
const int copy_body = (where != SAVED_IN_CACHE);
descriptors = microdescs_parse_from_string(s, eos, descriptors = microdescs_parse_from_string(s, eos,
allow_annotations, allow_annotations,
copy_body); where);
if (listed_at > 0) { if (listed_at > 0) {
SMARTLIST_FOREACH(descriptors, microdesc_t *, md, SMARTLIST_FOREACH(descriptors, microdesc_t *, md,
md->last_listed = listed_at); md->last_listed = listed_at);

View File

@ -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 /** 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 * <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 * annotations we recognize and ignore ones we don't.
* true, then strdup the bodies of the microdescriptors. Return all newly *
* 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. */ * parsed microdescriptors in a newly allocated smartlist_t. */
smartlist_t * smartlist_t *
microdescs_parse_from_string(const char *s, const char *eos, 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 *tokens;
smartlist_t *result; smartlist_t *result;
@ -4369,6 +4374,7 @@ microdescs_parse_from_string(const char *s, const char *eos,
const char *start = s; const char *start = s;
const char *start_of_next_microdesc; const char *start_of_next_microdesc;
int flags = allow_annotations ? TS_ANNOTATIONS_OK : 0; int flags = allow_annotations ? TS_ANNOTATIONS_OK : 0;
const int copy_body = (where != SAVED_IN_CACHE);
directory_token_t *tok; directory_token_t *tok;
@ -4398,6 +4404,7 @@ microdescs_parse_from_string(const char *s, const char *eos,
tor_assert(cp); tor_assert(cp);
md->bodylen = start_of_next_microdesc - cp; md->bodylen = start_of_next_microdesc - cp;
md->saved_location = where;
if (copy_body) if (copy_body)
md->body = tor_strndup(cp, md->bodylen); md->body = tor_strndup(cp, md->bodylen);
else else

View File

@ -64,7 +64,7 @@ ns_detached_signatures_t *networkstatus_parse_detached_signatures(
smartlist_t *microdescs_parse_from_string(const char *s, const char *eos, smartlist_t *microdescs_parse_from_string(const char *s, const char *eos,
int allow_annotations, int allow_annotations,
int copy_body); saved_location_t where);
authority_cert_t *authority_cert_parse_from_string(const char *s, authority_cert_t *authority_cert_parse_from_string(const char *s,
const char **end_of_string); const char **end_of_string);

View File

@ -226,8 +226,53 @@ test_md_cache(void *data)
tor_free(fn); 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[] = { struct testcase_t microdesc_tests[] = {
{ "cache", test_md_cache, TT_FORK, NULL, NULL }, { "cache", test_md_cache, TT_FORK, NULL, NULL },
{ "broken_cache", test_md_cache_broken, TT_FORK, NULL, NULL },
END_OF_TESTCASES END_OF_TESTCASES
}; };