From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.bugs Subject: bug#9397: lib-src integer and memory overflow issues Date: Sun, 28 Aug 2011 17:18:59 -0700 Organization: UCLA Computer Science Department Message-ID: <4E5ADAF3.1070301@cs.ucla.edu> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Trace: dough.gmane.org 1314577223 25647 80.91.229.12 (29 Aug 2011 00:20:23 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Mon, 29 Aug 2011 00:20:23 +0000 (UTC) To: 9397@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Mon Aug 29 02:20:19 2011 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([140.186.70.17]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1QxpaI-0001dD-Ne for geb-bug-gnu-emacs@m.gmane.org; Mon, 29 Aug 2011 02:20:15 +0200 Original-Received: from localhost ([::1]:38328 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QxpaI-0001cI-7p for geb-bug-gnu-emacs@m.gmane.org; Sun, 28 Aug 2011 20:20:14 -0400 Original-Received: from eggs.gnu.org ([140.186.70.92]:41859) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QxpaA-0001Yj-5O for bug-gnu-emacs@gnu.org; Sun, 28 Aug 2011 20:20:11 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qxpa8-0008WP-Eo for bug-gnu-emacs@gnu.org; Sun, 28 Aug 2011 20:20:06 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:45634) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qxpa8-0008WF-A4 for bug-gnu-emacs@gnu.org; Sun, 28 Aug 2011 20:20:04 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.69) (envelope-from ) id 1Qxpd0-00073f-HK; Sun, 28 Aug 2011 20:23:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Paul Eggert Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-To: owner@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 29 Aug 2011 00:23:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 9397 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch X-Debbugs-Original-To: bug-gnu-emacs@gnu.org Original-Received: via spool by submit@debbugs.gnu.org id=B.131457736527104 (code B ref -1); Mon, 29 Aug 2011 00:23:02 +0000 Original-Received: (at submit) by debbugs.gnu.org; 29 Aug 2011 00:22:45 +0000 Original-Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Qxpci-000736-9T for submit@debbugs.gnu.org; Sun, 28 Aug 2011 20:22:44 -0400 Original-Received: from eggs.gnu.org ([140.186.70.92]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1QxpcO-00072V-9w for submit@debbugs.gnu.org; Sun, 28 Aug 2011 20:22:39 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QxpZT-0008SO-Lt for submit@debbugs.gnu.org; Sun, 28 Aug 2011 20:19:25 -0400 Original-Received: from lists.gnu.org ([140.186.70.17]:54654) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QxpZT-0008SK-K2 for submit@debbugs.gnu.org; Sun, 28 Aug 2011 20:19:23 -0400 Original-Received: from eggs.gnu.org ([140.186.70.92]:41797) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QxpZR-0001Wx-LH for bug-gnu-emacs@gnu.org; Sun, 28 Aug 2011 20:19:23 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QxpZP-0008RO-DJ for bug-gnu-emacs@gnu.org; Sun, 28 Aug 2011 20:19:21 -0400 Original-Received: from smtp.cs.ucla.edu ([131.179.128.62]:43067) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QxpZO-0008PQ-PZ for bug-gnu-emacs@gnu.org; Sun, 28 Aug 2011 20:19:19 -0400 Original-Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id 269B2A60001 for ; Sun, 28 Aug 2011 17:19:09 -0700 (PDT) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Original-Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id xEFs0XNTsbte for ; Sun, 28 Aug 2011 17:19:07 -0700 (PDT) Original-Received: from [192.168.1.10] (pool-71-189-109-235.lsanca.fios.verizon.net [71.189.109.235]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id D7C5739E80D2 for ; Sun, 28 Aug 2011 17:19:07 -0700 (PDT) User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.20) Gecko/20110805 Thunderbird/3.1.12 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list Resent-Date: Sun, 28 Aug 2011 20:23:02 -0400 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 1) X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:50395 Archived-At: Package: Emacs Version: 24.0.50 Tags: patch Here's a patch to the Emacs trunk to fix some integer and memory overflow issues in lib-src programs. Currently, these programs can mess up on 64-bit hosts when given long strings, or uids greater than INT_MAX. In some cases the messups can occur on 32-bit hosts too. I plan to install this patch after a bit more testing. Here's an example bug fixed by the patch: "ctags -u -o FOO *.c" crashes on Fedora 14 x86-64 if the file name FOO is longer than 8192 bytes. === modified file 'lib-src/ChangeLog' --- lib-src/ChangeLog 2011-07-28 00:48:01 +0000 +++ lib-src/ChangeLog 2011-08-28 23:59:14 +0000 @@ -1,3 +1,38 @@ +2011-08-28 Paul Eggert + + Integer and memory overflow issues. + + * emacsclient.c (xmalloc): Accept size_t, not unsigned int, to + avoid potential buffer overflow issues on typical 64-bit hosts. + Return void *, not long *. + (get_current_dir_name): Report a failure, instead of looping + forever, if buffer size calculation overflows. Treat malloc + failures like realloc failures, as that has better behavior and is + more consistent. Do not check whether xmalloc returns NULL, as + that's not possible. + (message): Do not arbitrarily truncate message to 2048 bytes when + sending it to stderr; use vfprintf instead. + (get_server_config, set_local_socket) + (start_daemon_and_retry_set_socket): Do not alloca + arbitrarily-large buffers; that's not safe. + (get_server_config, set_local_socket): Do not use sprintf when its + result might not fit in 'int'. + (set_local_socket): Do not assume uid fits in 'int'. + + * etags.c (xmalloc, xrealloc): Accept size_t, not unsigned int, + to avoid potential buffer overflow issues on typical 64-bit hosts. + (whatlen_max): New static var. + (main): Avoid buffer overflow if subsidiary command length is + greater than BUFSIZ or 2*BUFSIZ + 20. Do not use sprintf when its + result might not fit in 'int'. + + * movemail.c (main): Do not use sprintf when its result might not fit + in 'int'. Instead, put the possibly-long file name into the + output of pfatal_with_name. + + * update-game-score.c: Include + (get_user_id): Do not assume uid fits in 'int'. Simplify. + 2011-07-28 Paul Eggert Assume freestanding C89 headers, string.h, stdlib.h. === modified file 'lib-src/emacsclient.c' --- lib-src/emacsclient.c 2011-07-02 15:07:57 +0000 +++ lib-src/emacsclient.c 2011-08-28 23:52:34 +0000 @@ -194,10 +194,10 @@ /* Like malloc but get fatal error if memory is exhausted. */ -static long * -xmalloc (unsigned int size) +static void * +xmalloc (size_t size) { - long *result = (long *) malloc (size); + void *result = malloc (size); if (result == NULL) { perror ("malloc"); @@ -250,32 +250,33 @@ ) { buf = (char *) xmalloc (strlen (pwd) + 1); - if (!buf) - return NULL; strcpy (buf, pwd); } #ifdef HAVE_GETCWD else { size_t buf_size = 1024; - buf = (char *) xmalloc (buf_size); - if (!buf) - return NULL; for (;;) { + int tmp_errno; + buf = malloc (buf_size); + if (! buf) + break; if (getcwd (buf, buf_size) == buf) break; - if (errno != ERANGE) + tmp_errno = errno; + free (buf); + if (tmp_errno != ERANGE) { - int tmp_errno = errno; - free (buf); errno = tmp_errno; return NULL; } buf_size *= 2; - buf = (char *) realloc (buf, buf_size); - if (!buf) - return NULL; + if (! buf_size) + { + errno = ENOMEM; + return NULL; + } } } #else @@ -283,8 +284,6 @@ { /* We need MAXPATHLEN here. */ buf = (char *) xmalloc (MAXPATHLEN + 1); - if (!buf) - return NULL; if (getwd (buf) == NULL) { int tmp_errno = errno; @@ -494,16 +493,17 @@ static void message (int is_error, const char *format, ...) { - char msg[2048]; va_list args; va_start (args, format); - vsprintf (msg, format, args); - va_end (args); #ifdef WINDOWSNT if (w32_window_app ()) { + char msg[2048]; + vsnprintf (msg, sizeof msg, format, args); + msg[sizeof msg - 1] = '\0'; + if (is_error) MessageBox (NULL, msg, "Emacsclient ERROR", MB_ICONERROR); else @@ -514,9 +514,11 @@ { FILE *f = is_error ? stderr : stdout; - fputs (msg, f); + vfprintf (f, format, args); fflush (f); } + + va_end (args); } /* Decode the options from argv and argc. @@ -959,18 +961,24 @@ if (home) { - char *path = alloca (strlen (home) + strlen (server_file) - + EXTRA_SPACE); - sprintf (path, "%s/.emacs.d/server/%s", home, server_file); + char *path = xmalloc (strlen (home) + strlen (server_file) + + EXTRA_SPACE); + strcpy (path, home); + strcat (path, "/.emacs.d/server/"); + strcat (path, server_file); config = fopen (path, "rb"); + free (path); } #ifdef WINDOWSNT if (!config && (home = egetenv ("APPDATA"))) { - char *path = alloca (strlen (home) + strlen (server_file) - + EXTRA_SPACE); - sprintf (path, "%s/.emacs.d/server/%s", home, server_file); + char *path = xmalloc (strlen (home) + strlen (server_file) + + EXTRA_SPACE); + strcpy (path, home); + strcat (path, "/.emacs.d/server/"); + strcat (path, server_file); config = fopen (path, "rb"); + free (path); } #endif } @@ -1243,6 +1251,8 @@ int saved_errno = 0; const char *server_name = "server"; const char *tmpdir IF_LINT ( = NULL); + char *tmpdir_storage = NULL; + char *socket_name_storage = NULL; if (socket_name && !strchr (socket_name, '/') && !strchr (socket_name, '\\')) @@ -1255,6 +1265,8 @@ if (default_sock) { + long uid = geteuid (); + ptrdiff_t tmpdirlen; tmpdir = egetenv ("TMPDIR"); if (!tmpdir) { @@ -1265,17 +1277,19 @@ size_t n = confstr (_CS_DARWIN_USER_TEMP_DIR, NULL, (size_t) 0); if (n > 0) { - tmpdir = alloca (n); + tmpdir = tmpdir_storage = xmalloc (n); confstr (_CS_DARWIN_USER_TEMP_DIR, tmpdir, n); } else #endif tmpdir = "/tmp"; } - socket_name = alloca (strlen (tmpdir) + strlen (server_name) - + EXTRA_SPACE); - sprintf (socket_name, "%s/emacs%d/%s", - tmpdir, (int) geteuid (), server_name); + tmpdirlen = strlen (tmpdir); + socket_name = socket_name_storage = + xmalloc (tmpdirlen + strlen (server_name) + EXTRA_SPACE); + strcpy (socket_name, tmpdir); + sprintf (socket_name + tmpdirlen, "/emacs%ld/", uid); + strcat (socket_name + tmpdirlen, server_name); } if (strlen (socket_name) < sizeof (server.sun_path)) @@ -1309,10 +1323,13 @@ if (pw && (pw->pw_uid != geteuid ())) { /* We're running under su, apparently. */ - socket_name = alloca (strlen (tmpdir) + strlen (server_name) - + EXTRA_SPACE); - sprintf (socket_name, "%s/emacs%d/%s", - tmpdir, (int) pw->pw_uid, server_name); + long uid = pw->pw_uid; + ptrdiff_t tmpdirlen = strlen (tmpdir); + socket_name = xmalloc (tmpdirlen + strlen (server_name) + + EXTRA_SPACE); + strcpy (socket_name, tmpdir); + sprintf (socket_name + tmpdirlen, "/emacs%ld/", uid); + strcat (socket_name + tmpdirlen, server_name); if (strlen (socket_name) < sizeof (server.sun_path)) strcpy (server.sun_path, socket_name); @@ -1322,6 +1339,7 @@ progname, socket_name); exit (EXIT_FAILURE); } + free (socket_name); sock_status = socket_status (server.sun_path); saved_errno = errno; @@ -1331,6 +1349,9 @@ } } + free (socket_name_storage); + free (tmpdir_storage); + switch (sock_status) { case 1: @@ -1526,8 +1547,8 @@ { /* Pass --daemon=socket_name as argument. */ const char *deq = "--daemon="; - char *daemon_arg = alloca (strlen (deq) - + strlen (socket_name) + 1); + char *daemon_arg = xmalloc (strlen (deq) + + strlen (socket_name) + 1); strcpy (daemon_arg, deq); strcat (daemon_arg, socket_name); d_argv[1] = daemon_arg; === modified file 'lib-src/etags.c' --- lib-src/etags.c 2011-07-07 01:32:56 +0000 +++ lib-src/etags.c 2011-08-28 23:55:41 +0000 @@ -414,8 +414,8 @@ static void canonicalize_filename (char *); static void linebuffer_init (linebuffer *); static void linebuffer_setlen (linebuffer *, int); -static PTR xmalloc (unsigned int); -static PTR xrealloc (char *, unsigned int); +static PTR xmalloc (size_t); +static PTR xrealloc (char *, size_t); static char searchar = '/'; /* use /.../ searches */ @@ -425,6 +425,7 @@ static char *cwd; /* current working directory */ static char *tagfiledir; /* directory of tagfile */ static FILE *tagf; /* ioptr for tags file */ +static ptrdiff_t whatlen_max; /* maximum length of any 'what' member */ static fdesc *fdhead; /* head of file description list */ static fdesc *curfdp; /* current file description */ @@ -1066,6 +1067,7 @@ int current_arg, file_count; linebuffer filename_lb; bool help_asked = FALSE; + ptrdiff_t len; char *optstring; int opt; @@ -1110,6 +1112,9 @@ /* This means that a file name has been seen. Record it. */ argbuffer[current_arg].arg_type = at_filename; argbuffer[current_arg].what = optarg; + len = strlen (optarg); + if (whatlen_max < len) + whatlen_max = len; ++current_arg; ++file_count; break; @@ -1118,6 +1123,9 @@ /* Parse standard input. Idea by Vivek . */ argbuffer[current_arg].arg_type = at_stdin; argbuffer[current_arg].what = optarg; + len = strlen (optarg); + if (whatlen_max < len) + whatlen_max = len; ++current_arg; ++file_count; if (parsing_stdin) @@ -1160,6 +1168,9 @@ case 'r': argbuffer[current_arg].arg_type = at_regexp; argbuffer[current_arg].what = optarg; + len = strlen (optarg); + if (whatlen_max < len) + whatlen_max = len; ++current_arg; break; case 'R': @@ -1198,6 +1209,9 @@ { argbuffer[current_arg].arg_type = at_filename; argbuffer[current_arg].what = argv[optind]; + len = strlen (argv[optind]); + if (whatlen_max < len) + whatlen_max = len; ++current_arg; ++file_count; } @@ -1331,7 +1345,9 @@ /* From here on, we are in (CTAGS && !cxref_style) */ if (update) { - char cmd[BUFSIZ]; + char *cmd = + xmalloc (strlen (tagfile) + whatlen_max + + sizeof "mv..OTAGS;fgrep -v '\t\t' OTAGS >;rm OTAGS"); for (i = 0; i < current_arg; ++i) { switch (argbuffer[i].arg_type) @@ -1342,12 +1358,17 @@ default: continue; /* the for loop */ } - sprintf (cmd, - "mv %s OTAGS;fgrep -v '\t%s\t' OTAGS >%s;rm OTAGS", - tagfile, argbuffer[i].what, tagfile); + strcpy (cmd, "mv "); + strcat (cmd, tagfile); + strcat (cmd, " OTAGS;fgrep -v '\t"); + strcat (cmd, argbuffer[i].what); + strcat (cmd, "\t' OTAGS >"); + strcat (cmd, tagfile); + strcat (cmd, ";rm OTAGS"); if (system (cmd) != EXIT_SUCCESS) fatal ("failed to execute shell command", (char *)NULL); } + free (cmd); append_to_tagfile = TRUE; } @@ -1363,11 +1384,14 @@ if (CTAGS) if (append_to_tagfile || update) { - char cmd[2*BUFSIZ+20]; + char *cmd = xmalloc (2 * strlen (tagfile) + sizeof "sort -u -o.."); /* Maybe these should be used: setenv ("LC_COLLATE", "C", 1); setenv ("LC_ALL", "C", 1); */ - sprintf (cmd, "sort -u -o %.*s %.*s", BUFSIZ, tagfile, BUFSIZ, tagfile); + strcpy (cmd, "sort -u -o "); + strcat (cmd, tagfile); + strcat (cmd, " "); + strcat (cmd, tagfile); exit (system (cmd)); } return EXIT_SUCCESS; @@ -6656,7 +6680,7 @@ /* Like malloc but get fatal error if memory is exhausted. */ static PTR -xmalloc (unsigned int size) +xmalloc (size_t size) { PTR result = (PTR) malloc (size); if (result == NULL) @@ -6665,7 +6689,7 @@ } static PTR -xrealloc (char *ptr, unsigned int size) +xrealloc (char *ptr, size_t size) { PTR result = (PTR) realloc (ptr, size); if (result == NULL) === modified file 'lib-src/movemail.c' --- lib-src/movemail.c 2011-07-07 01:32:56 +0000 +++ lib-src/movemail.c 2011-08-28 23:57:19 +0000 @@ -325,11 +325,10 @@ if (desc < 0) { int mkstemp_errno = errno; - char *message = (char *) xmalloc (strlen (tempname) + 50); - sprintf (message, "creating %s, which would become the lock file", - tempname); + error ("error while creating what would become the lock file", + 0, 0); errno = mkstemp_errno; - pfatal_with_name (message); + pfatal_with_name (tempname); } close (desc); === modified file 'lib-src/update-game-score.c' --- lib-src/update-game-score.c 2011-07-11 06:05:57 +0000 +++ lib-src/update-game-score.c 2011-08-28 23:59:14 +0000 @@ -35,6 +35,7 @@ #include #include +#include #include #include #include @@ -128,19 +129,13 @@ static char * get_user_id (void) { - char *name; struct passwd *buf = getpwuid (getuid ()); if (!buf) { - int count = 1; - int uid = (int) getuid (); - int tuid = uid; - while (tuid /= 10) - count++; - name = malloc (count+1); - if (!name) - return NULL; - sprintf (name, "%d", uid); + long uid = getuid (); + char *name = malloc (sizeof uid * CHAR_BIT / 3 + 1); + if (name) + sprintf (name, "%ld", uid); return name; } return buf->pw_name;