r13718@catbus: nickm | 2007-07-12 12:16:49 -0400

 Backport r10760: fix possible buffer overrun in natd code used by old BSDs.


svn:r10811

3 files changed, 14 insertions, 6 deletions

diff --git a/ChangeLog b/ChangeLog
index 7b5fd19b4a..3e198124bd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -11,6 +11,10 @@ Changes in version 0.1.2.15 - 2007-0?-??
       orconfig.h _before_ sys/types.h, so that we can get the expected
       definition of _FILE_OFFSET_BITS.  [Bugfix on 0.1.2.x]
 
+  o Major bugfixes (security):
+    - Fix a possible buffer overrun when using BSD natd support.  Bug found
+      by "Mr. Croup."
+
   o Minor bugfixes (directory)
     - Count the number of authorities that recommend each version
       correctly.  Previously, we were under-counting by 1.
diff --git a/doc/TODO.012 b/doc/TODO.012
index e1323de752..b3b4c80bb7 100644
--- a/doc/TODO.012
+++ b/doc/TODO.012
@@ -13,9 +13,9 @@ Backport items for 0.1.2:
   o r10563: use correct types with desc_digest_map.
   o r10566: build correctly on systems where size_t is bigger than ulong. 
   - r10579: new addsysuser implementation for osx (??)
-  - r10643: eventdns.c behavior fix for solaris.
+  o r10643: eventdns.c behavior fix for solaris.
   - r10730: Don't choose guards after any never-connected-to guard. (??)
-  - r10760: fix possible buffer overrun in old BSD natd code
+  o r10760: fix possible buffer overrun in old BSD natd code
   - r10790: Don't include reasons in destroy cells from the origin.
   - Some fix for bug 455.
 
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index cf34952f90..644ed54b64 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -1685,10 +1685,14 @@ connection_ap_process_natd(edge_connection_t *conn)
   }
 
   daddr = tbuf = &tmp_buf[0] + 6; /* after end of "[DEST " */
-  while (*tbuf != '\0' && *tbuf != ' ')
-    tbuf++;
-  *tbuf = '\0';
-  tbuf++;
+  if (!(tbuf = strchr(tbuf, ' '))) {
+    log_warn(LD_APP,"Natd handshake was ill-formed; closing. The client "
+             "said: %s",
+             escaped(tmp_buf));
+    connection_mark_unattached_ap(conn, END_STREAM_REASON_INVALID_NATD_DEST);
+    return -1;
+  }
+  *tbuf++ = '\0';
 
   /* pretend that a socks handshake completed so we don't try to
    * send a socks reply down a natd conn */