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

8 files changed, 82 insertions, 12 deletions

diff --git a/ChangeLog b/ChangeLog
index b53547456e..081cf8cc1d 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 f578c128f8..b87a19c664 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 c566bfe229..3ab72a5745 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 <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 *
diff --git a/src/common/util.h b/src/common/util.h
index 5d9a16030d..a9403f6a84 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 3d1f066414..66381bab9a 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 e66f45f02c..2c740a4892 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 3fa0f5efb6..e71cde49cb 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 bd1562e43b..6c77eb688e 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 <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>.
  */