bug#51327: 28.0.60; emacsclient warns about XDG_RUNTIME_DIR when starting daemon on-demand

[Jim Porter <jporterbugs@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1611 bytes --]

On 10/21/2021 9:58 PM, Jim Porter wrote:
> Normally, when running `emacsclient --alternate-editor=""' with no Emacs 
> server running, it will run `emacs --daemon' and then connect to it. In 
> Emacs 28, it will also issue the following warning:
> 
>    Should XDG_RUNTIME_DIR='/run/user/1000' be in the environment?
>    (Be careful: XDG_RUNTIME_DIR is security-related.)
> 
> However, XDG_RUNTIME_DIR *is* set in my environment, so it shouldn't be 
> warning me about it.
> 
> I believe this is due to the fix for bug#33847 (see commit 
> 007744dd0404d6febca88b00c22981cc630fb8c0). That bug asked for 
> emacsclient to look in both XDG_RUNTIME_DIR and TMPDIR to find the 
> server socket, in order to accommodate the case where `emacs --daemon' 
> is started when XDG_RUNTIME_DIR is unset, but *is* set when running 
> `emacsclient'.

Attached is a patch that should fix this by skipping the TMPDIR check 
whenever a) we have an alternate editor and b) XDG_RUNTIME_DIR is set. 
This has the benefit of supporting the use case in bug#33847 as well as 
users who start the Emacs daemon on-demand.

The only flaw I can think of with this method is that it would still be 
technically possible to perform a symlink attack against a user who runs 
`emacs --daemon' explicitly with XDG_RUNTIME_DIR set, and then runs 
`emacsclient' without an alternate editor set. However, this would 
require the attacker to be able to kill the `emacs --daemon' process 
somehow so that emacsclient falls back to looking in TMPDIR. I'm not 
sure that's a realistic attack vector, but I thought I'd mention it for 
completeness.

[-- Attachment #2: 0001-Prevent-symlink-attacks-in-emacsclient-when-an-alter.patch --]
[-- Type: text/plain, Size: 1148 bytes --]

From 6b8c7a9881b79254c618356a4dfa257812a6fe5c Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 30 Oct 2021 12:22:02 -0700
Subject: [PATCH] Prevent symlink attacks in emacsclient when an alternate
 editor is set

* lib-src/emacsclient.c (set_local_socket): Don't look in TMPDIR for a
socket if we have an alternate editor and XDG_RUNTIME_DIR is set
(Bug#51327).
---
 lib-src/emacsclient.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
index cff3cec2a7..1305226056 100644
--- a/lib-src/emacsclient.c
+++ b/lib-src/emacsclient.c
@@ -1466,7 +1466,10 @@ set_local_socket (char const *server_name)
 			 ? connect_socket (AT_FDCWD, sockname, s, 0)
 			 : ENAMETOOLONG);
 	}
-      if (sock_status == ENOENT)
+      /* Fall back to checking for a socket in TMPDIR unless we have
+	 an alternate editor and XDG_RUNTIME_DIR is set.  In that
+	 case, we want to bail out and spawn the alternate editor. */
+      if (!(xdg_runtime_dir && alternate_editor) && sock_status == ENOENT)
 	{
 	  char const *tmpdir = egetenv ("TMPDIR");
 	  if (tmpdir)
-- 
2.25.1