diff options
author | Paul Eggert <[email protected]> | 2011-08-28 16:52:34 -0700 |
---|---|---|
committer | Paul Eggert <[email protected]> | 2011-08-28 16:52:34 -0700 |
commit | 17107bb698f82bb5b9c8a732cd6b3faaad4d7af6 (patch) | |
tree | 5a0e1fb90617fbe9eb24ae763caed03c45daf948 /lib-src | |
parent | 5fc295a42feaef453b57010b826e589b9b9cb7dd (diff) |
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'.
Diffstat (limited to 'lib-src')
-rw-r--r-- | lib-src/ChangeLog | 21 | ||||
-rw-r--r-- | lib-src/emacsclient.c | 95 |
2 files changed, 79 insertions, 37 deletions
diff --git a/lib-src/ChangeLog b/lib-src/ChangeLog index c878d313b7..d056b1a4b8 100644 --- a/lib-src/ChangeLog +++ b/lib-src/ChangeLog @@ -1,3 +1,24 @@ +2011-08-28 Paul Eggert <[email protected]> + + 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'. + 2011-07-28 Paul Eggert <[email protected]> Assume freestanding C89 headers, string.h, stdlib.h. diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c index 2af139aee6..ece9dc65c4 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; |