zlib vulnerability could affect VNC

Tim Waugh twaugh "at" redhat.com
Tue, 12 Mar 2002 10:08:47 +0000


On Mon, Mar 11, 2002 at 09:46:44PM -0600, Mike Miller wrote:

> Has anyone seen this?  I hope it doesn't open a security hole in VNC.
> The article suggests that VNC is "potentially vulnerable."
> 
> http://www.linuxsecurity.com/articles/security_sources_article-4582.html

The reason is that many vendors were shipping VNC with zlib statically
linked in (which is the default).  In fact, a malicious user must
authenticate in order to be able to send compressed data.

However, VNC's own authentication has been shown to be prone to a
man-in-the-middle attack.

(Disregarding the fact that once you have authenticated to a VNC
server you can in general do things as the VNC server's user much more
easily than that..)

The other compelling reason to release VNC in this advisory was the
fact that there is an exploitable denial-of-service problem in the
mini-httpd that Xvnc provides.  Patch appended.

Tim.
*/

--- vnc_unixsrc/Xvnc/programs/Xserver/hw/vnc/httpd.c.httpd	Thu Apr 29 12:10:54 1999
+++ vnc_unixsrc/Xvnc/programs/Xserver/hw/vnc/httpd.c	Fri Feb  8 11:01:10 2002
@@ -48,11 +48,11 @@
 
 int httpListenSock = -1;
 int httpSock = -1;
-FILE* httpFP = NULL;
 
 #define BUF_SIZE 32768
 
 static char buf[BUF_SIZE];
+static size_t buf_filled; /* initially zero */
 
 
 /*
@@ -127,6 +127,7 @@
     }
 
     if (FD_ISSET(httpListenSock, &fds)) {
+	int flags;
 
 	if (httpSock >= 0) close(httpSock);
 
@@ -135,9 +136,13 @@
 	    rfbLogPerror("httpCheckFds: accept");
 	    return;
 	}
-	if ((httpFP = fdopen(httpSock, "r+")) == NULL) {
-	    rfbLogPerror("httpCheckFds: fdopen");
-	    close(httpSock);
+
+	flags = fcntl (httpSock, F_GETFL);
+
+	if (flags == -1 ||
+	fcntl (httpSock, F_SETFL, flags | O_NONBLOCK) == -1) {
+	    rfbLogPerror("httpCheckFds: fcntl");
+	    close (httpSock);
 	    httpSock = -1;
 	    return;
 	}
@@ -150,10 +155,10 @@
 static void
 httpCloseSock()
 {
-    fclose(httpFP);
-    httpFP = NULL;
+    close(httpSock);
     RemoveEnabledDevice(httpSock);
     httpSock = -1;
+    buf_filled = 0;
 }
 
 
@@ -170,7 +175,6 @@
     char *fname;
     int maxFnameLen;
     int fd;
-    Bool gotGet = FALSE;
     Bool performSubstitutions = FALSE;
     char str[256];
     struct passwd *user = getpwuid(getuid());;
@@ -184,66 +188,75 @@
     fname = &fullFname[strlen(fullFname)];
     maxFnameLen = 255 - strlen(fullFname);
 
-    buf[0] = '\0';
-
+    /* Read data from the HTTP client until we get a complete request. */
     while (1) {
+	ssize_t got = read (httpSock, buf + buf_filled,
+			    sizeof (buf) - buf_filled - 1);
+	if (got == -1) {
+	    if (errno == EAGAIN)
+		return;
 
-	/* Read lines from the HTTP client until a blank line.  The only
-	   line we need to parse is the line "GET <filename> ..." */
-
-	if (!fgets(buf, BUF_SIZE, httpFP)) {
-	    rfbLogPerror("httpProcessInput: fgets");
+	    rfbLogPerror("httpProcessInput: read");
 	    httpCloseSock();
 	    return;
 	}
 
-	if ((strcmp(buf,"\n") == 0) || (strcmp(buf,"\r\n") == 0)
-	    || (strcmp(buf,"\r") == 0) || (strcmp(buf,"\n\r") == 0))
-	    /* end of client request */
-	    break;
-
-	if (strncmp(buf, "GET ", 4) == 0) {
-	    gotGet = TRUE;
+	buf_filled += got;
+	buf[buf_filled] = '\0';
 
-	    if (strlen(buf) > maxFnameLen) {
-		rfbLog("GET line too long\n");
-		httpCloseSock();
-		return;
-	    }
+	/* Is it complete yet (is there a blank line)? */
+	if (strstr (buf, "\r\r") || strstr (buf, "\n\n") ||
+	    strstr (buf, "\r\n\r\n") || strstr (buf, "\n\r\n\r"))
+	    break;
+    }
 
-	    if (sscanf(buf, "GET %s HTTP/1.0", fname) != 1) {
-		rfbLog("couldn't parse GET line\n");
-		httpCloseSock();
-		return;
-	    }
+    /* Process the request. */
+    if (strncmp(buf, "GET ", 4)) {
+	rfbLog("no GET line\n");
+	httpCloseSock();
+	return;
+    } else {
+	/* Only use the first line. */
+	char *cp;
+	cp = strchr (buf, '\n');
+	if (!cp)
+	    cp = strchr (buf, '\r');
+	if (cp > buf && (*(cp - 1) == '\n' || *(cp - 1) == '\r'))
+	     cp--;
+	if (cp)
+	    *cp = '\0';
+    }
 
-	    if (fname[0] != '/') {
-		rfbLog("filename didn't begin with '/'\n");
-		WriteExact(httpSock, NOT_FOUND_STR, strlen(NOT_FOUND_STR));
-		httpCloseSock();
-		return;
-	    }
+    if (strlen(buf) > maxFnameLen) {
+	rfbLog("GET line too long\n");
+	httpCloseSock();
+	return;
+    }
 
-	    if (strchr(fname+1, '/') != NULL) {
-		rfbLog("asking for file in other directory\n");
-		WriteExact(httpSock, NOT_FOUND_STR, strlen(NOT_FOUND_STR));
-		httpCloseSock();
-		return;
-	    }
+    if (sscanf(buf, "GET %s HTTP/1.0", fname) != 1) {
+	rfbLog("couldn't parse GET line\n");
+	httpCloseSock();
+	return;
+    }
 
-	    getpeername(httpSock, (struct sockaddr *)&addr, &addrlen);
-	    rfbLog("httpd: get '%s' for %s\n", fname+1,
-		   inet_ntoa(addr.sin_addr));
-	    continue;
-	}
+    if (fname[0] != '/') {
+	rfbLog("filename didn't begin with '/'\n");
+	WriteExact(httpSock, NOT_FOUND_STR, strlen(NOT_FOUND_STR));
+	httpCloseSock();
+	return;
     }
 
-    if (!gotGet) {
-	rfbLog("no GET line\n");
+    if (strchr(fname+1, '/') != NULL) {
+	rfbLog("asking for file in other directory\n");
+	WriteExact(httpSock, NOT_FOUND_STR, strlen(NOT_FOUND_STR));
 	httpCloseSock();
 	return;
     }
 
+    getpeername(httpSock, (struct sockaddr *)&addr, &addrlen);
+    rfbLog("httpd: get '%s' for %s\n", fname+1,
+	   inet_ntoa(addr.sin_addr));
+
     /* If we were asked for '/', actually read the file index.vnc */
 
     if (strcmp(fname, "/") == 0) {
---------------------------------------------------------------------
To unsubscribe, mail majordomo "at" uk.research.att.com with the line:
'unsubscribe vnc-list' in the message BODY
See also: http://www.uk.research.att.com/vnc/intouch.html
---------------------------------------------------------------------