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

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

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

src/common/util.c

@@ -418,6 +418,36 @@ eat_whitespace(const char *s)
   }
 }
 
+/** Return a pointer to the first char of s before <b>eos</b> 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 *

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;

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 */

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,

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

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 <b>s</b> whose keyword is <b>keyword</b>; 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
  * <b>s</b>.
  */