[Emacs-diffs] master 3e5d775 1/4: emacsclient: fix symlink/socket race

[Paul Eggert]

branch: master
commit 3e5d7755454bea9b6ffd232b1d115c629cdb193d
Author: Paul Eggert <address@hidden>
Commit: Paul Eggert <address@hidden>

    emacsclient: fix symlink/socket race
    
    * lib-src/emacsclient.c (socket_status): New arg UID.
    All uses changed.
    (set_local_socket): Don’t create the unbound socket unless the
    initial sanity checks on the socket file succeed; this
    simplifies cleaning it up.  Check socket ownership again
    after connecting, to fix a race (Bug#33366).
---
 lib-src/emacsclient.c | 78 +++++++++++++++++++++++++++------------------------
 1 file changed, 41 insertions(+), 37 deletions(-)

diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
index ba72651..df44bc4 100644
--- a/lib-src/emacsclient.c
+++ b/lib-src/emacsclient.c
@@ -1079,20 +1079,21 @@ find_tty (const char **tty_type, const char **tty_name, 
bool noabort)
 
 #ifdef SOCKETS_IN_FILE_SYSTEM
 
-/* Three possibilities:
+/* Return the file status of NAME, ordinarily a socket.
+   It should be owned by UID.  Return one of the following:
   >0 - 'stat' failed with this errno value
   -1 - isn't owned by us
    0 - success: none of the above */
 
 static int
-socket_status (const char *name)
+socket_status (const char *name, uid_t uid)
 {
   struct stat statbfr;
 
   if (stat (name, &statbfr) != 0)
     return errno;
 
-  if (statbfr.st_uid != geteuid ())
+  if (statbfr.st_uid != uid)
     return -1;
 
   return 0;
@@ -1316,18 +1317,11 @@ set_local_socket (char const *server_name)
     struct sockaddr_un un;
     struct sockaddr sa;
   } server = {{ .sun_family = AF_UNIX }};
-
-  HSOCKET s = socket (AF_UNIX, SOCK_STREAM, 0);
-  if (s < 0)
-    {
-      message (true, "%s: socket: %s\n", progname, strerror (errno));
-      return INVALID_SOCKET;
-    }
-
   char *sockname = server.un.sun_path;
   enum { socknamesize = sizeof server.un.sun_path };
   int tmpdirlen = -1;
   int socknamelen = -1;
+  uid_t uid = geteuid ();
 
   if (strchr (server_name, '/')
       || (ISSLASH ('\\') && strchr (server_name, '\\')))
@@ -1359,7 +1353,7 @@ set_local_socket (char const *server_name)
                tmpdirlen = snprintf (sockname, socknamesize, "/tmp");
            }
          socknamelen = local_sockname (sockname, socknamesize, tmpdirlen,
-                                       geteuid (), server_name);
+                                       uid, server_name);
        }
     }
 
@@ -1370,7 +1364,7 @@ set_local_socket (char const *server_name)
     }
 
   /* See if the socket exists, and if it's owned by us. */
-  int sock_status = socket_status (sockname);
+  int sock_status = socket_status (sockname, uid);
   if (sock_status)
     {
       /* Failing that, see if LOGNAME or USER exist and differ from
@@ -1387,7 +1381,7 @@ set_local_socket (char const *server_name)
        {
          struct passwd *pw = getpwnam (user_name);
 
-         if (pw && (pw->pw_uid != geteuid ()))
+         if (pw && pw->pw_uid != uid)
            {
              /* We're running under su, apparently. */
              socknamelen = local_sockname (sockname, socknamesize, tmpdirlen,
@@ -1399,39 +1393,49 @@ set_local_socket (char const *server_name)
                  exit (EXIT_FAILURE);
                }
 
-             sock_status = socket_status (sockname);
+             sock_status = socket_status (sockname, uid);
            }
        }
     }
 
-  switch (sock_status)
+  if (sock_status == 0)
     {
-    case -1:
-      /* There's a socket, but it isn't owned by us.  */
-      message (true, "%s: Invalid socket owner\n", progname);
-      break;
+      HSOCKET s = socket (AF_UNIX, SOCK_STREAM, 0);
+      if (s < 0)
+       {
+         message (true, "%s: socket: %s\n", progname, strerror (errno));
+         return INVALID_SOCKET;
+       }
+      if (connect (s, &server.sa, sizeof server.un) != 0)
+       {
+         message (true, "%s: connect: %s\n", progname, strerror (errno));
+         CLOSE_SOCKET (s);
+         return INVALID_SOCKET;
+       }
 
-    case 0:
-      if (connect (s, &server.sa, sizeof server.un) == 0)
+      struct stat connect_stat;
+      if (fstat (s, &connect_stat) != 0)
+       sock_status = errno;
+      else if (connect_stat.st_uid == uid)
        return s;
-      message (true, "%s: connect: %s\n", progname, strerror (errno));
-      break;
-
-    default:
-      /* 'stat' failed.  */
-      if (sock_status == ENOENT)
-       message (true,
-                ("%s: can't find socket; have you started the server?\n"
-                 "%s: To start the server in Emacs,"
-                 " type \"M-x server-start\".\n"),
-                progname, progname);
       else
-       message (true, "%s: can't stat %s: %s\n",
-                progname, sockname, strerror (sock_status));
-      break;
+       sock_status = -1;
+
+      CLOSE_SOCKET (s);
     }
 
-  CLOSE_SOCKET (s);
+  if (sock_status < 0)
+    message (true, "%s: Invalid socket owner\n", progname);
+  else if (sock_status == ENOENT)
+    message (true,
+            ("%s: can't find socket; have you started the server?\n"
+             "%s: To start the server in Emacs,"
+             " type \"M-x server-start\".\n"),
+            progname, progname);
+  else
+    message (true, "%s: can't stat %s: %s\n",
+            progname, sockname, strerror (sock_status));
+
   return INVALID_SOCKET;
 }
 #endif /* SOCKETS_IN_FILE_SYSTEM */