aboutsummaryrefslogtreecommitdiffstats
path: root/lib-src
diff options
context:
space:
mode:
authorPaul Eggert <[email protected]>2011-08-28 16:52:34 -0700
committerPaul Eggert <[email protected]>2011-08-28 16:52:34 -0700
commit17107bb698f82bb5b9c8a732cd6b3faaad4d7af6 (patch)
tree5a0e1fb90617fbe9eb24ae763caed03c45daf948 /lib-src
parent5fc295a42feaef453b57010b826e589b9b9cb7dd (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/ChangeLog21
-rw-r--r--lib-src/emacsclient.c95
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;