[or-cvs] Resolve FIXME items: fix assert failure on malformed socks4...

[Nick Mathewson]

Update of /home/or/cvsroot/src/or
In directory moria.mit.edu:/tmp/cvs-serv32414/src/or

Modified Files:
	buffers.c 
Log Message:
Resolve FIXME items: fix assert failure on malformed socks4a qreuests. (bug reported by Anna Shubina wrt old Netscapes)

Index: buffers.c
===================================================================
RCS file: /home/or/cvsroot/src/or/buffers.c,v
retrieving revision 1.113
retrieving revision 1.114
diff -u -d -r1.113 -r1.114
--- buffers.c	7 Nov 2004 22:18:00 -0000	1.113
+++ buffers.c	10 Nov 2004 14:26:34 -0000	1.114
@@ -578,13 +578,15 @@
         log_fn(LOG_DEBUG,"socks4: Username not here yet.");
         return 0;
       }
+      tor_assert(next < buf->mem+buf->datalen);
 
-      startaddr = next+1;
+      startaddr = NULL;
       if(socks4_prot != socks4a && !have_warned_about_unsafe_socks) {
         log_fn(LOG_WARN,"Your application (using socks4 on port %d) is giving Tor only an IP address. Applications that do DNS resolves themselves may leak information. Consider using Socks4A (e.g. via privoxy or socat) instead.", req->port);
 //      have_warned_about_unsafe_socks = 1; // (for now, warn every time)
       }
-      if(socks4_prot == socks4a) {
+      if(socks4_prot == socks4a && next+1 < buf->mem+buf->datalen) {
+        startaddr = next+1;
         next = memchr(startaddr, 0, buf->mem+buf->datalen-startaddr);
         if(!next) {
           log_fn(LOG_DEBUG,"socks4: Destaddr not here yet.");
@@ -594,13 +596,11 @@
           log_fn(LOG_WARN,"socks4: Destaddr too long. Rejecting.");
           return -1;
         }
+        tor_assert(next < buf->mem+buf->datalen);
       }
       log_fn(LOG_DEBUG,"socks4: Everything is here. Success.");
-      strlcpy(req->address, socks4_prot == socks4 ? tmpbuf : startaddr,
+      strlcpy(req->address, startaddr ? startaddr : tmpbuf,
               sizeof(req->address));
-      /* XXX on very old netscapes (socks4) the next line triggers an
-       * assert, because next-buf->mem+1 is greater than buf->datalen.
-       */
       buf_remove_from_front(buf, next-buf->mem+1); /* next points to the final \0 on inbuf */
       return 1;