[or-cvs] r10846: Backport a minimal fix for bug 455: strndup a NUL-terminated (in tor/branches/tor-0_1_2-patches: . doc src/or)

[nickm at seul.org]

Author: nickm
Date: 2007-07-16 13:46:31 -0400 (Mon, 16 Jul 2007)
New Revision: 10846

Modified:
   tor/branches/tor-0_1_2-patches/
   tor/branches/tor-0_1_2-patches/ChangeLog
   tor/branches/tor-0_1_2-patches/doc/TODO.012
   tor/branches/tor-0_1_2-patches/src/or/routerparse.c
Log:
 r13786 at catbus:  nickm | 2007-07-16 13:46:28 -0400
 Backport a minimal fix for bug 455: strndup a NUL-terminated copy of each router descriptor before trying to parse it.  If this slows us down a lot, we will need to reconsider, but it seems far safer than the more sophisticated stuff we are trying to do to fix 455 on trunk.



Property changes on: tor/branches/tor-0_1_2-patches
___________________________________________________________________
 svk:merge ticket from /tor/012 [r13786] on 8246c3cf-6607-4228-993b-4d95d33730f1

Modified: tor/branches/tor-0_1_2-patches/ChangeLog
===================================================================
--- tor/branches/tor-0_1_2-patches/ChangeLog	2007-07-16 17:27:35 UTC (rev 10845)
+++ tor/branches/tor-0_1_2-patches/ChangeLog	2007-07-16 17:46:31 UTC (rev 10846)
@@ -10,6 +10,9 @@
     - Fix eventdns.c behavior on Solaris: It is critical to include
       orconfig.h _before_ sys/types.h, so that we can get the expected
       definition of _FILE_OFFSET_BITS.
+    - When the cached-routers file is an even multiple of the page size,
+      don't run off the end and crash.  (Fixes bug 455; based on idea
+      from croup.)
 
   o Major bugfixes (security):
     - Fix a possible buffer overrun when using BSD natd support.

Modified: tor/branches/tor-0_1_2-patches/doc/TODO.012
===================================================================
--- tor/branches/tor-0_1_2-patches/doc/TODO.012	2007-07-16 17:27:35 UTC (rev 10845)
+++ tor/branches/tor-0_1_2-patches/doc/TODO.012	2007-07-16 17:46:31 UTC (rev 10846)
@@ -16,7 +16,7 @@
   o r10730: Don't choose guards after any never-connected-to guard.
   o r10760: fix possible buffer overrun in old BSD natd code
   o r10790: Don't include reasons in destroy cells from the origin.
-  - Some fix for bug 455.
+  o Some fix for bug 455.
   o r10830 if nick thinks it's a real fix
   - r10835 if nick thinks it's a real fix
 

Modified: tor/branches/tor-0_1_2-patches/src/or/routerparse.c
===================================================================
--- tor/branches/tor-0_1_2-patches/src/or/routerparse.c	2007-07-16 17:27:35 UTC (rev 10845)
+++ tor/branches/tor-0_1_2-patches/src/or/routerparse.c	2007-07-16 17:46:31 UTC (rev 10846)
@@ -649,6 +649,8 @@
 {
   routerinfo_t *router;
   const char *end, *cp, *start;
+  char *buf;
+  size_t buf_len;
 
   tor_assert(s);
   tor_assert(*s);
@@ -690,14 +692,23 @@
     /* cp now points to the first \n before the last non-blank line in this
      * descriptor */
 
+    if (eos - cp < 25) /* not long enough to hold an "end signature" */
+      break;
+
     if (strcmpstart(cp, "\n-----END SIGNATURE-----\n")) {
       log_info(LD_DIR, "Ignoring truncated router descriptor.");
       *s = end;
       continue;
     }
 
-    router = router_parse_entry_from_string(*s, end,
+    /* router_parse_entry_from_string isn't necessarily safe if the string
+     * is non-NUL-terminated.  This fix is a workaround for the stable
+     * series only;  */
+    buf_len = end-*s;
+    buf = tor_strndup(*s, buf_len); /* nul-terminates the copy. */
+    router = router_parse_entry_from_string(buf, buf+buf_len,
                                             saved_location != SAVED_IN_CACHE);
+    tor_free(buf);
 
     if (!router) {
       log_warn(LD_DIR, "Error reading router; skipping");