Re: [PATCH 21.5] gnuclient buffer overflow fixes

PATCH 21.5 The gnuclient.c part of this patch will not apply to 21.4. I can make a new patch for 21.4 if desired, though. Here are fixes for a few, um, undesirable features found with a static checker. lib-src/ChangeLog addition: 2006-08-08 Jerry James <james(a)xemacs.org&gt; * 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. xemacs-21.5 source patch: Diff command: cvs -q diff -uN Files affected: lib-src/gnuclient.c =================================================================== RCS lib-src/gnuslib.c =================================================================== RCS Index: lib-src/gnuslib.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/lib-src/gnuslib.c,v retrieving revision 1.12 diff -d -u -r1.12 gnuslib.c --- lib-src/gnuslib.c 2001/08/13 04:45:48 1.12 +++ lib-src/gnuslib.c 2006/08/08 20:27:59 ＠＠ -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); Index: lib-src/gnuclient.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/lib-src/gnuclient.c,v retrieving revision 1.27 diff -d -u -r1.27 gnuclient.c --- lib-src/gnuclient.c 2004/12/06 23:23:41 1.27 +++ lib-src/gnuclient.c 2006/08/08 20:27:59 ＠＠ -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) -- Jerry James, Assistant Professor james(a)xemacs.org Computer Science Department http://www.cs.usu.edu/~jerry/ Utah State University