changeset 3556:2161ac78b41e

[xemacs-hg @ 2006-08-11 17:37:17 by james] Prevent potential buffer overflows. Also enable use of shutdown() on linux. <m37j1jez4n.fsf@jerrypc.cs.usu.edu>
author james
date Fri, 11 Aug 2006 17:37:18 +0000
parents db783273b399
children 88f032ab6753
files lib-src/ChangeLog lib-src/gnuclient.c lib-src/gnuslib.c
diffstat 3 files changed, 28 insertions(+), 17 deletions(-) [+]
line wrap: on
line diff
--- a/lib-src/ChangeLog	Fri Aug 11 01:30:23 2006 +0000
+++ b/lib-src/ChangeLog	Fri Aug 11 17:37:18 2006 +0000
@@ -1,3 +1,13 @@
+2006-08-08  Jerry James  <james@xemacs.org>
+
+	* gnuslib.c (disconnect_from_server): shutdown() has been fine on
+	Linux for a long time now; use it.  Also, don't use length to
+	access the buffer unless it is positive, not just nonzero.
+	* gnuclient.c (filename_expand): Initialize the last array element
+	to get a valid C string in case of overflow.  Use strncat to avoid
+	buffer overruns.
+	* gnuclient.c (main): Use strncpy to avoid buffer overruns.
+
 2006-07-21  Stephen J. Turnbull  <stephen@xemacs.org>
 
 	* etags.c: Xemacs -> XEmacs.
--- a/lib-src/gnuclient.c	Fri Aug 11 01:30:23 2006 +0000
+++ b/lib-src/gnuclient.c	Fri Aug 11 17:37:18 2006 +0000
@@ -187,7 +187,7 @@
 #endif
 
   int len;
-  fullpath[0] = '\0';
+  fullpath[0] = fullpath[QXE_PATH_MAX] = '\0';
 
 #ifdef  CYGWIN
   /*
@@ -200,7 +200,7 @@
   if (filename[0] && filename[0] == '/')
      {
        /* Absolute (unix-style) pathname.  Do nothing */
-       strcat (fullpath, filename);
+       strncat (fullpath, filename, QXE_PATH_MAX);
      }
   else
     {
@@ -208,15 +208,18 @@
        and prepend it.  FIXME: need to fix the case of DOS paths like
        "\foo", where we need to get the current drive. */
 
-      strcat (fullpath, get_current_working_directory ());
+      strncat (fullpath, get_current_working_directory (), QXE_PATH_MAX);
       len = strlen (fullpath);
 
-      if (len > 0 && fullpath[len-1] == '/')	/* trailing slash already? */
-	;					/* yep */
-      else
-	strcat (fullpath, "/");		/* nope, append trailing slash */
+      /* If no trailing slash, add one */
+      if (len <= 0 || (fullpath[len - 1] != '/' && len < QXE_PATH_MAX))
+	{
+	  strcat (fullpath, "/");
+	  len++;
+	}
+
       /* Don't forget to add the filename! */
-      strcat (fullpath,filename);
+      strncat (fullpath, filename, QXE_PATH_MAX - len);
     }
 } /* filename_expand */
 
@@ -435,7 +438,7 @@
 		  break;
 		case 'r':
 		  GET_ARGUMENT (remotearg, "-r");
-		  strcpy (remotepath, remotearg);
+		  strncpy (remotepath, remotearg, QXE_PATH_MAX);
 		  rflg = 1;
 		  break;
 #endif /* INTERNET_DOMAIN_SOCKETS */
@@ -590,7 +593,7 @@
 					 * to this machine */
 	      if ((ptr = getenv ("GNU_NODE")) != NULL)
 		/* user specified a path */
-		strcpy (remotepath, ptr);
+		strncpy (remotepath, ptr, QXE_PATH_MAX);
 	    }
 #if 0  /* This is really bogus... re-enable it if you must have it! */
 #if defined (hp9000s300) || defined (hp9000s800)
--- a/lib-src/gnuslib.c	Fri Aug 11 01:30:23 2006 +0000
+++ b/lib-src/gnuslib.c	Fri Aug 11 17:37:18 2006 +0000
@@ -409,13 +409,11 @@
 
   send_string(s,EOT_STR);		/* make sure server gets string */
 
-#if !defined (linux)  && !defined (_SCO_DS) 
+#ifndef _SCO_DS
   /*
-   * shutdown is completely hozed under linux. If s is a unix domain socket,
-   * you'll get EOPNOTSUPP back from it. If s is an internet socket, you get
-   * a broken pipe when you try to read a bit later. The latter
-   * problem is fixed for linux versions >= 1.1.46, but the problem
-   * with unix sockets persists. Sigh.
+   * There used to be a comment here complaining about ancient Linux
+   * versions.  It is no longer relevant.  I don't know why _SCO_DS is
+   * verboten here, as the original comment did not say.
    */
 
   if (shutdown(s,1) == -1) {
@@ -434,7 +432,7 @@
 #else
   while ((length = read(s,buffer,GSERV_BUFSZ)) > 0 ||
       (length == -1 && errno == EINTR)) {
-    if (length) {
+    if (length > 0) {
       buffer[length] = '\0';
       if (echo) {
 	fputs(buffer,stdout);