From 007744dd0404d6febca88b00c22981cc630fb8c0 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Fri, 23 Jul 2021 13:33:21 +0200 Subject: Redo emacsclient socket symlink-attack checking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * admin/merge-gnulib (GNULIB_MODULES): Add file-has-acl. * lib/file-has-acl.c: New file, copied from Gnulib. * lib/gnulib.mk.in, m4/gnulib-comp.m4: Regenerate. * lib-src/emacsclient.c: Include acl.h, for file_has_acl. (O_PATH): Default to O_SEARCH, which is good enough here. (union local_sockaddr): New type. (socket_status): Remove, replacing with ... (connect_socket): New function. All callers changed. This function checks for ownership and permissions issues with the parent directory of the socket file, instead of checking the owner of the socket (which does not help security). (socknamesize): Move to file scope. (local_sockname): New arg S. No need to pass socknamesize. UID arg is now uid_t. All callers changed. Get file descriptor of parent directory of socket, to foil some symlink attacks. Do not follow symlinks to that directory. (set_local_socket): Create the socket here instead of on each attempt to connect it. Fall back from XDG_RUNTIME_DIR to /tmp only if the former fails due to ENOENT. Adjust permission-failure diagnostic to match changed behavior. This addresses Bug#33847, which complained about emacsclient in a safer XDG environment not connecting to an Emacs server running in a less-safe enviroment outside XDG. The patch fixes a longstanding issue with emacsclient permission checking. It’s ineffective to look at the permission of the socket file itself; on some platforms, these permissions are ignored anyway. What matters are the permissions on the parent directory of the socket file, as these are what make symlink attacks possible. Change the permissions check accordingly, and also refuse to follow symlinks to that parent directory. These changes make it OK for emacsclient to fall back from XDG_RUNTIME_DIR to the traditionally less-safe /tmp/emacsNNNN directories, since /tmp is universally sticky nowadays. --- m4/gnulib-comp.m4 | 3 +++ 1 file changed, 3 insertions(+) (limited to 'm4') diff --git a/m4/gnulib-comp.m4 b/m4/gnulib-comp.m4 index cd6f7b4bbdf..05e7faa9937 100644 --- a/m4/gnulib-comp.m4 +++ b/m4/gnulib-comp.m4 @@ -89,6 +89,7 @@ AC_DEFUN([gl_EARLY], # Code from module fcntl: # Code from module fcntl-h: # Code from module fdopendir: + # Code from module file-has-acl: # Code from module filemode: # Code from module filename: # Code from module filevercmp: @@ -287,6 +288,7 @@ AC_DEFUN([gl_INIT], fi gl_DIRENT_MODULE_INDICATOR([fdopendir]) gl_MODULE_INDICATOR([fdopendir]) + gl_FILE_HAS_ACL gl_FILEMODE AC_C_FLEXIBLE_ARRAY_MEMBER gl_FUNC_FPENDING @@ -1045,6 +1047,7 @@ AC_DEFUN([gl_FILE_LIST], [ lib/fcntl.c lib/fcntl.in.h lib/fdopendir.c + lib/file-has-acl.c lib/filemode.c lib/filemode.h lib/filename.h -- cgit v1.2.1