5 files changed, 135 insertions, 61 deletions

diff --git a/lib-src/ChangeLog b/lib-src/ChangeLog
index c878d313b70..9041eae8bbe 100644
--- a/lib-src/ChangeLog
+++ b/lib-src/ChangeLog
@@ -1,3 +1,38 @@
+2011-09-04  Paul Eggert  <eggert@cs.ucla.edu>
+
+        Integer and memory overflow issues (Bug#9397).
+
+        * 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 <limits.h>
+        (get_user_id): Do not assume uid fits in 'int'.  Simplify.
+
 2011-07-28  Paul Eggert  <eggert@cs.ucla.edu>
 
         Assume freestanding C89 headers, string.h, stdlib.h.
diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
index 2af139aee6d..ece9dc65c49 100644
--- a/lib-src/emacsclient.c
+++ b/lib-src/emacsclient.c
@@ -194,10 +194,10 @@ struct option longopts[] =
 
 /* 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 @@ get_current_dir_name (void)
       )
     {
       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 @@ get_current_dir_name (void)
     {
       /* 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, const char *, ...) ATTRIBUTE_FORMAT_PRINTF (2, 3);
 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 @@ message (int is_error, const char *format, ...)
     {
       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 @@ get_server_config (struct sockaddr_in *server, char *authentication)
 
       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 @@ set_local_socket (void)
     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 @@ set_local_socket (void)
 
     if (default_sock)
       {
+        long uid = geteuid ();
+        ptrdiff_t tmpdirlen;
         tmpdir = egetenv ("TMPDIR");
         if (!tmpdir)
           {
@@ -1265,17 +1277,19 @@ set_local_socket (void)
             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 @@ set_local_socket (void)
             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 @@ set_local_socket (void)
                              progname, socket_name);
                     exit (EXIT_FAILURE);
                   }
+                free (socket_name);
 
                 sock_status = socket_status (server.sun_path);
                 saved_errno = errno;
@@ -1331,6 +1349,9 @@ set_local_socket (void)
           }
       }
 
+    free (socket_name_storage);
+    free (tmpdir_storage);
+
     switch (sock_status)
       {
       case 1:
@@ -1526,8 +1547,8 @@ start_daemon_and_retry_set_socket (void)
         {
           /* 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;
diff --git a/lib-src/etags.c b/lib-src/etags.c
index 522c54ee4a5..9d920565804 100644
--- a/lib-src/etags.c
+++ b/lib-src/etags.c
@@ -414,8 +414,8 @@ static bool filename_is_absolute (char *f);
 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 *progname;		/* name this program was invoked with */
 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 @@ main (int argc, char **argv)
   int current_arg, file_count;
   linebuffer filename_lb;
   bool help_asked = FALSE;
+  ptrdiff_t len;
  char *optstring;
  int opt;
 
@@ -1110,6 +1112,9 @@ main (int argc, char **argv)
         /* 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 @@ main (int argc, char **argv)
         /* Parse standard input.  Idea by Vivek <vivek@etla.org>. */
         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 @@ main (int argc, char **argv)
       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 @@ main (int argc, char **argv)
     {
       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 @@ main (int argc, char **argv)
   /* 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 @@ main (int argc, char **argv)
             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 @@ main (int argc, char **argv)
   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 @@ linebuffer_setlen (linebuffer *lbp, int toksize)
 
 /* 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 @@ xmalloc (unsigned int size)
 }
 
 static PTR
-xrealloc (char *ptr, unsigned int size)
+xrealloc (char *ptr, size_t size)
 {
   PTR result = (PTR) realloc (ptr, size);
   if (result == NULL)
diff --git a/lib-src/movemail.c b/lib-src/movemail.c
index d70c655adec..097bf23c202 100644
--- a/lib-src/movemail.c
+++ b/lib-src/movemail.c
@@ -325,11 +325,10 @@ main (int argc, char **argv)
           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);
 
diff --git a/lib-src/update-game-score.c b/lib-src/update-game-score.c
index 2a89379aefe..9fba51a33de 100644
--- a/lib-src/update-game-score.c
+++ b/lib-src/update-game-score.c
@@ -35,6 +35,7 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include <unistd.h>
 #include <errno.h>
+#include <limits.h>
 #include <string.h>
 #include <stdlib.h>
 #include <stdio.h>
@@ -128,19 +129,13 @@ lose_syserr (const char *msg)
 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;