From fa64904306888059cefe50429ce6f1502f2a19c6 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 24 May 2007 15:48:53 +0000 Subject: [PATCH] r12912@catbus: nickm | 2007-05-24 11:48:49 -0400 Backport minimal parts of r10192 (fix bugs found by Benedikt) and r10248 (handle lack of nul at end of mmap). svn:r10301 --- ChangeLog | 2 ++ src/common/compat.h | 8 ++++++++ src/common/util.c | 30 ++++++++++++++++++++++++++++++ src/common/util.h | 1 + src/or/directory.c | 2 +- src/or/or.h | 3 ++- src/or/routerlist.c | 8 +++++--- src/or/routerparse.c | 40 +++++++++++++++++++++++++++++++++------- 8 files changed, 82 insertions(+), 12 deletions(-) diff --git a/ChangeLog b/ChangeLog index b53547456..081cf8cc1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -20,6 +20,8 @@ Changes in version 0.1.2.14 - 2007-05-23 - If all of our dirservers have given us bad or no networkstatuses lately, then stop hammering them once per minute even when we think they're failed. Fixes another part of bug 422. + - Tighten router parsing rules. + - Avoid segfaults when reading from mmaped descriptor file. o Minor bugfixes: - Actually set the purpose correctly for descriptors inserted with diff --git a/src/common/compat.h b/src/common/compat.h index f578c128f..b87a19c66 100644 --- a/src/common/compat.h +++ b/src/common/compat.h @@ -154,6 +154,14 @@ int tor_vsnprintf(char *str, size_t size, const char *format, va_list args) const void *tor_memmem(const void *haystack, size_t hlen, const void *needle, size_t nlen) ATTR_PURE ATTR_NONNULL((1,3)); +static const void *tor_memstr(const void *haystack, size_t hlen, + const char *needle) ATTR_PURE ATTR_NONNULL((1,3)); +static INLINE const void * +tor_memstr(const void *haystack, size_t hlen, const char *needle) +{ + return tor_memmem(haystack, hlen, needle, strlen(needle)); +} + #define TOR_ISALPHA(c) isalpha((int)(unsigned char)(c)) #define TOR_ISALNUM(c) isalnum((int)(unsigned char)(c)) #define TOR_ISSPACE(c) isspace((int)(unsigned char)(c)) diff --git a/src/common/util.c b/src/common/util.c index c566bfe22..3ab72a574 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -418,6 +418,36 @@ eat_whitespace(const char *s) } } +/** Return a pointer to the first char of s before eos that is not + * whitespace and not a comment, or to the terminating NUL or eos if no such + * character exists. + */ +const char * +eat_whitespace_eos(const char *s, const char *eos) +{ + tor_assert(s); + tor_assert(eos && s <= eos); + + while (s < eos) { + switch (*s) { + case '\0': + default: + return s; + case ' ': + case '\t': + case '\n': + case '\r': + ++s; + break; + case '#': + ++s; + while (s < eos && *s && *s != '\n') + ++s; + } + } + return s; +} + /** Return a pointer to the first char of s that is not a space or a tab, * or to the terminating NUL if no such character exists. */ const char * diff --git a/src/common/util.h b/src/common/util.h index 5d9a16030..a9403f6a8 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -158,6 +158,7 @@ uint64_t tor_parse_uint64(const char *s, int base, uint64_t min, uint64_t max, int *ok, char **next); const char *hex_str(const char *from, size_t fromlen) ATTR_NONNULL((1)); const char *eat_whitespace(const char *s) ATTR_PURE; +const char *eat_whitespace_eos(const char *s, const char *eos) ATTR_PURE; const char *eat_whitespace_no_nl(const char *s) ATTR_PURE; const char *find_whitespace(const char *s) ATTR_PURE; int tor_mem_is_zero(const char *mem, size_t len) ATTR_PURE; diff --git a/src/or/directory.c b/src/or/directory.c index 3d1f06641..66381bab9 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -1158,7 +1158,7 @@ connection_dir_client_reached_eof(dir_connection_t *conn) if (which || (conn->requested_resource && !strcmpstart(conn->requested_resource, "all"))) { /* as we learn from them, we remove them from 'which' */ - router_load_routers_from_string(body, SAVED_NOWHERE, which); + router_load_routers_from_string(body, body_len, SAVED_NOWHERE, which); directory_info_has_arrived(time(NULL), 0); } if (which) { /* mark remaining ones as failed */ diff --git a/src/or/or.h b/src/or/or.h index e66f45f02..2c740a489 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -2943,7 +2943,7 @@ int router_add_to_routerlist(routerinfo_t *router, const char **msg, int from_cache, int from_fetch); int router_load_single_router(const char *s, uint8_t purpose, const char **msg); -void router_load_routers_from_string(const char *s, +void router_load_routers_from_string(const char *s, size_t len, saved_location_t saved_location, smartlist_t *requested_fingerprints); typedef enum { @@ -3024,6 +3024,7 @@ int router_append_dirobj_signature(char *buf, size_t buf_len, const char *digest, crypto_pk_env_t *private_key); int router_parse_list_from_string(const char **s, + const char *eos, smartlist_t *dest, saved_location_t saved_location); int router_parse_routerlist_from_directory(const char *s, diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 3fa0f5efb..e71cde49c 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -373,6 +373,7 @@ router_reload_router_list(void) if (routerlist->mmap_descriptors) { router_store_len = routerlist->mmap_descriptors->size; router_load_routers_from_string(routerlist->mmap_descriptors->data, + routerlist->mmap_descriptors->size, SAVED_IN_CACHE, NULL); } @@ -381,7 +382,7 @@ router_reload_router_list(void) if (file_status(fname) == FN_FILE) contents = read_file_to_str(fname, RFTS_BIN|RFTS_IGNORE_MISSING, NULL); if (contents) { - router_load_routers_from_string(contents, + router_load_routers_from_string(contents, strlen(contents), SAVED_IN_JOURNAL, NULL); tor_free(contents); } @@ -2302,7 +2303,8 @@ router_load_single_router(const char *s, uint8_t purpose, const char **msg) * fingerprint from the list. */ void -router_load_routers_from_string(const char *s, saved_location_t saved_location, +router_load_routers_from_string(const char *s, size_t len, + saved_location_t saved_location, smartlist_t *requested_fingerprints) { smartlist_t *routers = smartlist_create(), *changed = smartlist_create(); @@ -2310,7 +2312,7 @@ router_load_routers_from_string(const char *s, saved_location_t saved_location, const char *msg; int from_cache = (saved_location != SAVED_NOWHERE); - router_parse_list_from_string(&s, routers, saved_location); + router_parse_list_from_string(&s, s+len, routers, saved_location); routers_update_status_from_networkstatus(routers, !from_cache); diff --git a/src/or/routerparse.c b/src/or/routerparse.c index bd1562e43..6c77eb688 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -163,6 +163,8 @@ static void token_free(directory_token_t *tok); static smartlist_t *find_all_exitpolicy(smartlist_t *s); static directory_token_t *find_first_by_keyword(smartlist_t *s, directory_keyword keyword); +static directory_token_t *find_last_by_keyword(smartlist_t *s, + directory_keyword keyword); static int tokenize_string(const char *start, const char *end, smartlist_t *out, where_syntax where); static directory_token_t *get_next_token(const char **s, where_syntax where); @@ -641,7 +643,8 @@ check_directory_signature(const char *digest, * Returns 0 on success and -1 on failure. */ int -router_parse_list_from_string(const char **s, smartlist_t *dest, +router_parse_list_from_string(const char **s, const char *eos, + smartlist_t *dest, saved_location_t saved_location) { routerinfo_t *router; @@ -652,19 +655,25 @@ router_parse_list_from_string(const char **s, smartlist_t *dest, tor_assert(dest); start = *s; + if (!eos) + eos = *s + strlen(*s); + while (1) { - *s = eat_whitespace(*s); + *s = eat_whitespace_eos(*s, eos); + if (eos - *s < 32) /* not long enough to hold a descriptor. */ + break; + /* Don't start parsing the rest of *s unless it contains a router. */ if (strcmpstart(*s, "router ")!=0) break; - if ((end = strstr(*s+1, "\nrouter "))) { + if ((end = tor_memstr(*s+1, eos-(*s+1), "\nrouter "))) { cp = end; end++; - } else if ((end = strstr(*s+1, "\ndirectory-signature"))) { + } else if ((end = tor_memstr(*s+1, eos-(*s+1), "\ndirectory-signature"))) { cp = end; end++; } else { - cp = end = *s+strlen(*s); + cp = end = eos; } while (cp > *s && (!*cp || TOR_ISSPACE(*cp))) @@ -938,7 +947,12 @@ router_parse_entry_from_string(const char *s, const char *end, log_warn(LD_DIR, "Missing router signature"); goto err; } - if (strcmp(tok->object_type, "SIGNATURE") || tok->object_size != 128) { + if (tok != find_last_by_keyword(tokens, K_ROUTER_SIGNATURE)) { + log_warn(LD_DIR, "Multiple signatures on one router. That's not ok."); + goto err; + } + if (!tok->object_type || + strcmp(tok->object_type, "SIGNATURE") || tok->object_size != 128) { log_warn(LD_DIR, "Bad object type or length on router signature"); goto err; } @@ -1637,7 +1651,7 @@ get_next_token(const char **s, where_syntax where) } *s = eat_whitespace(*s); if (strcmpstart(*s, "-----BEGIN ")) { - goto done_tokenizing; + goto check_obj; } obstart = *s; *s += 11; /* length of "-----BEGIN ". */ @@ -1673,6 +1687,7 @@ get_next_token(const char **s, where_syntax where) } *s += i+6; } + check_obj: switch (o_syn) { case NO_OBJ: @@ -1735,6 +1750,17 @@ find_first_by_keyword(smartlist_t *s, directory_keyword keyword) return NULL; } +/** Find the last token in s whose keyword is keyword; return + * NULL if no such keyword is found. + */ +static directory_token_t * +find_last_by_keyword(smartlist_t *s, directory_keyword keyword) +{ + directory_token_t *last = NULL; + SMARTLIST_FOREACH(s, directory_token_t *, t, if (t->tp == keyword) last = t); + return last; +} + /** Return a newly allocated smartlist of all accept or reject tokens in * s. */