Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Po Lu]

Ulrich Müller <ulm@gentoo.org> writes:

>  Categories=Network;Email;
>  Comment=GNU Emacs is an extensible, customizable text editor - and more
> -Exec=sh -c "exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$1\\\\\\")\\"" sh %u
> +# We want to pass the following commands to the shell wrapper:
> +# u=${1//\\/\\\\}; u=${u//\"/\\\"}; exec emacsclient --alternate-editor= --display="$DISPLAY" --eval "(message-mailto \"$u\")"
> +# Special chars '"', '$', and '\' must be escaped as '\\"', '\\$', and '\\\\'.
> +Exec=bash -c "u=\\${1//\\\\\\\\/\\\\\\\\\\\\\\\\}; u=\\${u//\\\\\\"/\\\\\\\\\\\\\\"}; exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" bash %u
>  Icon=emacs
>  Name=Emacs (Mail, Client)
>  MimeType=x-scheme-handler/mailto;
> @@ -13,7 +16,7 @@ Actions=new-window;new-instance;
>  
>  [Desktop Action new-window]
>  Name=New Window
> -Exec=sh -c "exec emacsclient --alternate-editor= --create-frame --eval \\"(message-mailto \\\\\\"\\$1\\\\\\")\\"" sh %u
> +Exec=bash -c "u=\\${1//\\\\\\\\/\\\\\\\\\\\\\\\\}; u=\\${u//\\\\\\"/\\\\\\\\\\\\\\"}; exec emacsclient --alternate-editor= --create-frame --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" bash %u
>  
>  [Desktop Action new-instance]
>  Name=New Instance

What if the system in question has no bash?  This is not a theoretical
question, because I have access to one system which does have .desktop
files, but only csh, /bin/sh (which is useless), and ksh93.

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Ulrich Mueller]

>>>>> On Wed, 08 Mar 2023, Po Lu wrote:

> Ulrich Müller <ulm@gentoo.org> writes:
>> Categories=Network;Email;
>> Comment=GNU Emacs is an extensible, customizable text editor - and more
>> -Exec=sh -c "exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$1\\\\\\")\\"" sh %u
>> +# We want to pass the following commands to the shell wrapper:
>> +# u=${1//\\/\\\\}; u=${u//\"/\\\"}; exec emacsclient --alternate-editor= --display="$DISPLAY" --eval "(message-mailto \"$u\")"
>> +# Special chars '"', '$', and '\' must be escaped as '\\"', '\\$', and '\\\\'.
>> +Exec=bash -c "u=\\${1//\\\\\\\\/\\\\\\\\\\\\\\\\}; u=\\${u//\\\\\\"/\\\\\\\\\\\\\\"}; exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" bash %u
>> Icon=emacs
>> Name=Emacs (Mail, Client)
>> MimeType=x-scheme-handler/mailto;
>> @@ -13,7 +16,7 @@ Actions=new-window;new-instance;
>> 
>> [Desktop Action new-window]
>> Name=New Window
>> -Exec=sh -c "exec emacsclient --alternate-editor= --create-frame --eval \\"(message-mailto \\\\\\"\\$1\\\\\\")\\"" sh %u
>> +Exec=bash -c "u=\\${1//\\\\\\\\/\\\\\\\\\\\\\\\\}; u=\\${u//\\\\\\"/\\\\\\\\\\\\\\"}; exec emacsclient --alternate-editor= --create-frame --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" bash %u
>> 
>> [Desktop Action new-instance]
>> Name=New Instance

> What if the system in question has no bash?

Then the desktop file won't work, obviously. The problem is that
${PARAMETER//PATTERN/STRING} substitution is not available in POSIX
parameter expansion. So with POSIX sh, an external program (e.g. sed)
would have to be called.

The long term solution (suggested by Stefan Monnier) might be to add
a --funcall option to emacsclient. Then there would be no need for a
shell wrapper, in the first place.

Should the Makefile skip installation of emacsclient-mail.desktop
when bash isn't available on the system?

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Po Lu]

Ulrich Mueller <ulm@gentoo.org> writes:

> Then the desktop file won't work, obviously. The problem is that
> ${PARAMETER//PATTERN/STRING} substitution is not available in POSIX
> parameter expansion. So with POSIX sh, an external program (e.g. sed)
> would have to be called.
>
> The long term solution (suggested by Stefan Monnier) might be to add
> a --funcall option to emacsclient. Then there would be no need for a
> shell wrapper, in the first place.
>
> Should the Makefile skip installation of emacsclient-mail.desktop
> when bash isn't available on the system?

Could we install this change not on emacs-29, but on master?

I don't think the problem it solves is severe, nor a regression from
Emacs 28.  It is rather a minor nusiance with certain URLs.

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Ulrich Mueller]

>>>>> On Wed, 08 Mar 2023, Po Lu wrote:

> Ulrich Mueller <ulm@gentoo.org> writes:
>> Then the desktop file won't work, obviously. The problem is that
>> ${PARAMETER//PATTERN/STRING} substitution is not available in POSIX
>> parameter expansion. So with POSIX sh, an external program (e.g. sed)
>> would have to be called.
>> 
>> The long term solution (suggested by Stefan Monnier) might be to add
>> a --funcall option to emacsclient. Then there would be no need for a
>> shell wrapper, in the first place.
>> 
>> Should the Makefile skip installation of emacsclient-mail.desktop
>> when bash isn't available on the system?

> Could we install this change not on emacs-29, but on master?

> I don't think the problem it solves is severe, nor a regression from
> Emacs 28.  It is rather a minor nusiance with certain URLs.

Seriously? It is a vulnerability that allows remote injection of
arbitrary Elisp code through a crafted "mailto" URI.

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Po Lu]

Ulrich Mueller <ulm@gentoo.org> writes:

>>>>>> On Wed, 08 Mar 2023, Po Lu wrote:
>
>> Ulrich Mueller <ulm@gentoo.org> writes:
>>> Then the desktop file won't work, obviously. The problem is that
>>> ${PARAMETER//PATTERN/STRING} substitution is not available in POSIX
>>> parameter expansion. So with POSIX sh, an external program (e.g. sed)
>>> would have to be called.
>>> 
>>> The long term solution (suggested by Stefan Monnier) might be to add
>>> a --funcall option to emacsclient. Then there would be no need for a
>>> shell wrapper, in the first place.
>>> 
>>> Should the Makefile skip installation of emacsclient-mail.desktop
>>> when bash isn't available on the system?
>
>> Could we install this change not on emacs-29, but on master?
>
>> I don't think the problem it solves is severe, nor a regression from
>> Emacs 28.  It is rather a minor nusiance with certain URLs.
>
> Seriously? It is a vulnerability that allows remote injection of
> arbitrary Elisp code through a crafted "mailto" URI.

For it to be a vulnerability, you will have to click such mailto URIs in
your web browser without first reading them, and some nasty person will
have to specifically create URIs that run insidious Emacs Lisp code.

How about something simpler: one can copy a command to download malware
from the Internet, then paste it into a shell buffer.  Let's remove a
serious command injection vulnerability, ``M-x shell'', from Emacs 29!
While we're at it, how about `interprogram-paste-function' as well?

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Ulrich Mueller]

>>>>> On Wed, 08 Mar 2023, Po Lu wrote:

> For it to be a vulnerability, you will have to click such mailto URIs in
> your web browser without first reading them, and some nasty person will
> have to specifically create URIs that run insidious Emacs Lisp code.

> How about something simpler: one can copy a command to download malware
> from the Internet, then paste it into a shell buffer.  Let's remove a
> serious command injection vulnerability, ``M-x shell'', from Emacs 29!
> While we're at it, how about `interprogram-paste-function' as well?

No, it doesn't work that way. :) When it comes to vulnerabilities, it is
all about expectations.

If I execute a program (shell code, binary, etc.) that I find somewhere
in the Internet, then I know that it will execute some code, and that I
must trust its source that it doesn't do anything malicious.

OTOH, I don't have that expectation when I click on a mailto hyperlink.

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Po Lu]

Ulrich Mueller <ulm@gentoo.org> writes:

>>>>>> On Wed, 08 Mar 2023, Po Lu wrote:
>
>> For it to be a vulnerability, you will have to click such mailto URIs in
>> your web browser without first reading them, and some nasty person will
>> have to specifically create URIs that run insidious Emacs Lisp code.
>
>> How about something simpler: one can copy a command to download malware
>> from the Internet, then paste it into a shell buffer.  Let's remove a
>> serious command injection vulnerability, ``M-x shell'', from Emacs 29!
>> While we're at it, how about `interprogram-paste-function' as well?
>
> No, it doesn't work that way. :) When it comes to vulnerabilities, it is
> all about expectations.
>
> If I execute a program (shell code, binary, etc.) that I find somewhere
> in the Internet, then I know that it will execute some code, and that I
> must trust its source that it doesn't do anything malicious.
>
> OTOH, I don't have that expectation when I click on a mailto hyperlink.

Before you click a hyperlink, the URL to which it points pops up on the
lower left corner.  You have every opportunity to see that it doesn't do
anything nasty.

And again, you're already making the very brazen assumption that some
nasty person out there stands ready to take advantage of this bug.

Please, fix this so it works without bash, or remove it from emacs-29.
Once the pretest comes out, I plan to ask many coworkers to try it out.
Many of their systems use the Korn shell and do not have bash.

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Robert Pluim]

>>>>> On Wed, 08 Mar 2023 08:15:48 +0100, Ulrich Mueller <ulm@gentoo.org> said:

>>>>> On Wed, 08 Mar 2023, Po Lu wrote:
    >> Ulrich Mueller <ulm@gentoo.org> writes:
    >>> Then the desktop file won't work, obviously. The problem is that
    >>> ${PARAMETER//PATTERN/STRING} substitution is not available in POSIX
    >>> parameter expansion. So with POSIX sh, an external program (e.g. sed)
    >>> would have to be called.
    >>> 
    >>> The long term solution (suggested by Stefan Monnier) might be to add
    >>> a --funcall option to emacsclient. Then there would be no need for a
    >>> shell wrapper, in the first place.
    >>> 
    >>> Should the Makefile skip installation of emacsclient-mail.desktop
    >>> when bash isn't available on the system?

Then youʼd require every distro builder to have bash available in
their build setup

    >> Could we install this change not on emacs-29, but on master?

    >> I don't think the problem it solves is severe, nor a regression from
    >> Emacs 28.  It is rather a minor nusiance with certain URLs.

    Ulrich> Seriously? It is a vulnerability that allows remote injection of
    Ulrich> arbitrary Elisp code through a crafted "mailto" URI.

Itʼs certainly not a regression, but it is fairly serious. We could
mitigate it somewhat by adding '--funcall', I guess.

The last time --funcall was discussed, there was no consensus on how
arguments should be handled, so Iʼve just gone ahead and implemented
one variant. We could add any restrictions we like on the server side,
it currently just disallows direct `eval'

Not for emacs-29, I think.

Robert
-- 

diff --git c/lib-src/emacsclient.c i/lib-src/emacsclient.c
index 698bf9b50ae..d408af6658f 100644
--- c/lib-src/emacsclient.c
+++ i/lib-src/emacsclient.c
@@ -113,6 +113,9 @@ #define DEFAULT_TIMEOUT (30)
 /* True means don't print values returned from emacs. --suppress-output.  */
 static bool suppress_output;
 
+/* True means arg is a function to be called.  --funcall.  */
+static bool funcall;
+
 /* True means args are expressions to be evaluated.  --eval.  */
 static bool eval;
 
@@ -168,6 +171,7 @@ #define DEFAULT_TIMEOUT (30)
   { "no-wait",	no_argument,	   NULL, 'n' },
   { "quiet",	no_argument,	   NULL, 'q' },
   { "suppress-output", no_argument, NULL, 'u' },
+  { "funcall",	no_argument,	   NULL, 'l' },
   { "eval",	no_argument,	   NULL, 'e' },
   { "help",	no_argument,	   NULL, 'H' },
   { "version",	no_argument,	   NULL, 'V' },
@@ -191,7 +195,7 @@ #define DEFAULT_TIMEOUT (30)
 /* Short options, in the same order as the corresponding long options.
    There is no '-p' short option.  */
 static char const shortopts[] =
-  "nqueHVtca:F:w:"
+  "nquleHVtca:F:w:"
 #ifdef SOCKETS_IN_FILE_SYSTEM
   "s:"
 #endif
@@ -548,6 +552,10 @@ decode_options (int argc, char **argv)
 	    }
 	  break;
 
+	case 'l':
+	  funcall = true;
+	  break;
+
 	case 'e':
 	  eval = true;
 	  break;
@@ -689,6 +697,7 @@ print_help_and_exit (void)
 ", "\
 -F ALIST, --frame-parameters=ALIST\n\
 			Set the parameters of a new frame\n\
+-l, --funcall    	Evaluate the FILE argument as function to call\n\
 -e, --eval    		Evaluate the FILE arguments as ELisp expressions\n\
 -n, --no-wait		Don't wait for the server to return\n\
 -w, --timeout=SECONDS	Seconds to wait before timing out\n\
@@ -2067,11 +2076,28 @@ main (int argc, char **argv)
   if (create_frame && !tty)
     send_to_emacs (emacs_socket, "-window-system ");
 
+  bool funcall_sent = false;
+
   if (optind < argc)
     {
       for (int i = optind; i < argc; i++)
 	{
 
+	  if (funcall)
+            {
+	      /* funcall applies the function to all subsequent
+		 arguments, unlike eval, which evaluates each
+		 expression individually.  */
+	      if (!funcall_sent)
+		{
+		  send_to_emacs (emacs_socket, "-funcall ");
+		  funcall_sent = true;
+		}
+              quote_argument (emacs_socket, argv[i]);
+              send_to_emacs (emacs_socket, " ");
+              continue;
+            }
+
 	  if (eval)
             {
               /* Don't prepend cwd or anything like that.  */
@@ -2139,7 +2165,7 @@ main (int argc, char **argv)
   send_to_emacs (emacs_socket, "\n");
 
   /* Wait for an answer. */
-  if (!eval && !tty && !nowait && !quiet && 0 <= process_grouping ())
+  if (!funcall && !eval && !tty && !nowait && !quiet && 0 <= process_grouping ())
     {
       printf ("Waiting for Emacs...");
       skiplf = false;
diff --git c/lisp/server.el i/lisp/server.el
index eaf24a770e4..e5ea2b14b3b 100644
--- c/lisp/server.el
+++ i/lisp/server.el
@@ -1295,6 +1295,29 @@ server-process-filter
                                proc))
                  (setq filepos nil))
 
+                ;; -funcall FUNCTION:  Call the function, and pass it any subsequent command line args
+                ("-funcall"
+                 (if use-current-frame
+                     (setq use-current-frame 'always))
+                 (let ((fun (pop args-left))
+                       args real-fun)
+                   (if coding-system
+                       (setq fun (decode-coding-string fun coding-system)))
+                   (when fun
+                     (setq real-fun (intern-soft fun))
+                     (when (and (fboundp real-fun)
+                                (not (eq real-fun 'eval)))
+                       (setq args (mapcar
+                                   (lambda (a)
+                                     (if coding-system
+                                         (setq a (decode-coding-string a coding-system)))
+                                     a)
+                                   args-left))
+                       (setq args-left nil)
+                       (push (lambda () (apply real-fun args))
+                             commands))))
+                 (setq filepos nil))
+
                 ;; -eval EXPR:  Evaluate a Lisp expression.
                 ("-eval"
                  (if use-current-frame

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Ulrich Mueller]

>>>>> On Wed, 08 Mar 2023, Po Lu wrote:

> Please, fix this so it works without bash, or remove it from emacs-29.
> Once the pretest comes out, I plan to ask many coworkers to try it out.
> Many of their systems use the Korn shell and do not have bash.

Sorry, but I've installed this on emacs-29 with an explicit ack from
both Eli and Stefan.

An alternative solution would be to drop emacsclient-mail.desktop
altogether, since this desktop file isn't part of any core
functionality. It could be readded once emacsclient has gained a
--funcall argument, so that arguments can be passed in a sane way.

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Robert Pluim]

>>>>> On Wed, 08 Mar 2023 11:39:32 +0100, Ulrich Mueller <ulm@gentoo.org> said:

>>>>> On Wed, 08 Mar 2023, Po Lu wrote:
    >> Please, fix this so it works without bash, or remove it from emacs-29.
    >> Once the pretest comes out, I plan to ask many coworkers to try it out.
    >> Many of their systems use the Korn shell and do not have bash.

    Ulrich> Sorry, but I've installed this on emacs-29 with an explicit ack from
    Ulrich> both Eli and Stefan.

Fair enough, but is there no way to get it to use `sed' instead?

    Ulrich> An alternative solution would be to drop emacsclient-mail.desktop
    Ulrich> altogether, since this desktop file isn't part of any core
    Ulrich> functionality. It could be readded once emacsclient has gained a
    Ulrich> --funcall argument, so that arguments can be passed in a sane way.

No, I think too many people like using that desktop file for us to
yank it like that.

Robert
-- 

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Po Lu]

Ulrich Mueller <ulm@gentoo.org> writes:

> Sorry, but I've installed this on emacs-29 with an explicit ack from
> both Eli and Stefan.

Why was this considered okay for emacs-29?

IMHO we should stop kow-towing to the information security people who
make a huge fuss over every single bug, especially bugs like this one
which only show up when you specifically try to trigger them.

Proprietary JavaScript routinely does things far more nasty and
malicious than a hyperlink that can be read before being clicked.

> An alternative solution would be to drop emacsclient-mail.desktop
> altogether, since this desktop file isn't part of any core
> functionality. It could be readded once emacsclient has gained a
> --funcall argument, so that arguments can be passed in a sane way.

Or perhaps Emacs 29 can forgo this change entirely.  Why would anyone
click a URL containing suspicious looking Lisp code, and who would
actually try to do nasty things with such URLs?

If you have to go out of your way to trigger a bug in a branch that is
supposed to be stable, then fixing it can wait.

Just my two cents.  Feel free to disregard them if you want, but keep in
mind it will result in many angry users.

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Ulrich Mueller]

>>>>> On Wed, 08 Mar 2023, Robert Pluim wrote:

Ulrich> Sorry, but I've installed this on emacs-29 with an explicit ack from
Ulrich> both Eli and Stefan.

> Fair enough, but is there no way to get it to use `sed' instead?

Sure, something like this (not yet tested):

-Exec=bash -c "u=\\${1//\\\\\\\\/\\\\\\\\\\\\\\\\}; u=\\${u//\\\\\\"/\\\\\\\\\\\\\\"}; exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" bash %u
+Exec=sh -c "u=\\$\\(echo \\"\\$1\\" | sed \\'s/[\\\\\\"]/\\\\\\\\\\&/g\\'\\); exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u

Sorry, but it doesn't get less ugly. :) Also, it will now call _two_
external programs. Is this acceptable, and is it guaranteed that sed
will be available on users' systems?

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Ulrich Mueller]

>>>>> On Wed, 08 Mar 2023, Ulrich Mueller wrote:

> Sure, something like this (not yet tested):

> -Exec=bash -c "u=\\${1//\\\\\\\\/\\\\\\\\\\\\\\\\}; u=\\${u//\\\\\\"/\\\\\\\\\\\\\\"}; exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" bash %u
> +Exec=sh -c "u=\\$\\(echo \\"\\$1\\" | sed \\'s/[\\\\\\"]/\\\\\\\\\\&/g\\'\\); exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u

> Sorry, but it doesn't get less ugly. :) Also, it will now call _two_
> external programs. Is this acceptable, and is it guaranteed that sed
> will be available on users' systems?

In fact, only '"', '$', and '\' need to be backslash-escaped.
Patch included below, works fine here.

----- 8< -----
From 1e1f128c5ca0c888dae4651bcb3d556737e3a207 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ulrich=20M=C3=BCller?= <ulm@gentoo.org>
Date: Wed, 8 Mar 2023 12:23:58 +0100
Subject: [PATCH] Avoid using bash in the emacsclient desktop file

* etc/emacsclient-mail.desktop (Exec): Use sh and sed instead
of bash, because the latter may not be available everywhere.
---
 etc/emacsclient-mail.desktop | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/etc/emacsclient-mail.desktop b/etc/emacsclient-mail.desktop
index 49c6f99f317..0a2420ddead 100644
--- a/etc/emacsclient-mail.desktop
+++ b/etc/emacsclient-mail.desktop
@@ -2,9 +2,9 @@
 Categories=Network;Email;
 Comment=GNU Emacs is an extensible, customizable text editor - and more
 # We want to pass the following commands to the shell wrapper:
-# u=${1//\\/\\\\}; u=${u//\"/\\\"}; exec emacsclient --alternate-editor= --display="$DISPLAY" --eval "(message-mailto \"$u\")"
+# u=$(echo "$1" | sed 's/[\"]/\\&/g'); exec emacsclient --alternate-editor= --display="$DISPLAY" --eval "(message-mailto \"$u\")"
 # Special chars '"', '$', and '\' must be escaped as '\\"', '\\$', and '\\\\'.
-Exec=bash -c "u=\\${1//\\\\\\\\/\\\\\\\\\\\\\\\\}; u=\\${u//\\\\\\"/\\\\\\\\\\\\\\"}; exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" bash %u
+Exec=sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u
 Icon=emacs
 Name=Emacs (Mail, Client)
 MimeType=x-scheme-handler/mailto;
@@ -16,7 +16,7 @@ Actions=new-window;new-instance;
 
 [Desktop Action new-window]
 Name=New Window
-Exec=bash -c "u=\\${1//\\\\\\\\/\\\\\\\\\\\\\\\\}; u=\\${u//\\\\\\"/\\\\\\\\\\\\\\"}; exec emacsclient --alternate-editor= --create-frame --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" bash %u
+Exec=sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec emacsclient --alternate-editor= --create-frame --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u
 
 [Desktop Action new-instance]
 Name=New Instance
-- 
2.39.2

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Po Lu]

Ulrich Mueller <ulm@gentoo.org> writes:

> Is this acceptable, and is it guaranteed that sed will be available on
> users' systems?

Yes to both, thanks.

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Ulrich Mueller]

>>>>> On Wed, 08 Mar 2023, Po Lu wrote:

> IMHO we should stop kow-towing to the information security people who
> make a huge fuss over every single bug, especially bugs like this one
> which only show up when you specifically try to trigger them.

> Proprietary JavaScript routinely does things far more nasty and
> malicious than a hyperlink that can be read before being clicked.

> Or perhaps Emacs 29 can forgo this change entirely.  Why would anyone
> click a URL containing suspicious looking Lisp code, and who would
> actually try to do nasty things with such URLs?

> If you have to go out of your way to trigger a bug in a branch that is
> supposed to be stable, then fixing it can wait.

Wow. :)

At least you seem to agree that the current behaviour is a bug.

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Robert Pluim]

>>>>> On Wed, 08 Mar 2023 12:29:13 +0100, Ulrich Mueller <ulm@gentoo.org> said:

>>>>> On Wed, 08 Mar 2023, Ulrich Mueller wrote:
    >> Sure, something like this (not yet tested):

    >> -Exec=bash -c "u=\\${1//\\\\\\\\/\\\\\\\\\\\\\\\\}; u=\\${u//\\\\\\"/\\\\\\\\\\\\\\"}; exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" bash %u
    >> +Exec=sh -c "u=\\$\\(echo \\"\\$1\\" | sed \\'s/[\\\\\\"]/\\\\\\\\\\&/g\\'\\); exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u

    >> Sorry, but it doesn't get less ugly. :) Also, it will now call _two_
    >> external programs. Is this acceptable, and is it guaranteed that sed
    >> will be available on users' systems?

`sed' is POSIX mandatory, and `bash' is not. And `echo' is mandatory
as well, since youʼre also using that :-)

    Ulrich> In fact, only '"', '$', and '\' need to be backslash-escaped.
    Ulrich> Patch included below, works fine here.

That looks better. We can then add `--funcall' to emacs-30, and revisit
this there.

Thanks

Robert
-- 

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Ulrich Mueller]

>>>>> On Wed, 08 Mar 2023, Robert Pluim wrote:

> +  { "funcall",	no_argument,	   NULL, 'l' },

I understand that options -f, -F, -u, -n, -c, and -a are already taken.
Still, I find -l for the short option confusing, because emacs itself
uses -f for --funcall and -l for --load.

Do we need a short option?

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Po Lu <luangruo@yahoo.com>,  emacs-devel@gnu.org
> Date: Wed, 08 Mar 2023 11:37:06 +0100
> 
> Itʼs certainly not a regression, but it is fairly serious. We could
> mitigate it somewhat by adding '--funcall', I guess.
> 
> The last time --funcall was discussed, there was no consensus on how
> arguments should be handled, so Iʼve just gone ahead and implemented
> one variant. We could add any restrictions we like on the server side,
> it currently just disallows direct `eval'
> 
> Not for emacs-29, I think.

Indeed, not for emacs-29.  So if requiring Bash is not going to fly,
we need to find a different way to fix the original problem.  Adding a
general-purpose command-line option to emacsclient, which also
requires a change in the client-server protocol, is out of the
question for emacs-29.

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Eli Zaretskii]

> From: Ulrich Mueller <ulm@gentoo.org>
> Cc: emacs-devel@gnu.org
> Date: Wed, 08 Mar 2023 11:39:32 +0100
> 
> An alternative solution would be to drop emacsclient-mail.desktop
> altogether, since this desktop file isn't part of any core
> functionality.

No, that's also impossible.  It's too late to remove it.  (I was
against adding it in the first place, but that's water under the
bridge at this time.)

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Wed, 08 Mar 2023 18:58:47 +0800
> 
> Ulrich Mueller <ulm@gentoo.org> writes:
> 
> > Sorry, but I've installed this on emacs-29 with an explicit ack from
> > both Eli and Stefan.
> 
> Why was this considered okay for emacs-29?

Because I didn't imagine that Bash could be missing on a GNU/Linux
system.

> IMHO we should stop kow-towing to the information security people who
> make a huge fuss over every single bug, especially bugs like this one
> which only show up when you specifically try to trigger them.

Good luck with that!

This is a losing battle, so I suggest you save your time and energy
for more productive discussions.

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Eli Zaretskii]

> From: Ulrich Mueller <ulm@gentoo.org>
> Cc: Po Lu <luangruo@yahoo.com>,  emacs-devel@gnu.org
> Date: Wed, 08 Mar 2023 12:08:42 +0100
> 
> >>>>> On Wed, 08 Mar 2023, Robert Pluim wrote:
> 
> > Fair enough, but is there no way to get it to use `sed' instead?
> 
> Sure, something like this (not yet tested):
> 
> -Exec=bash -c "u=\\${1//\\\\\\\\/\\\\\\\\\\\\\\\\}; u=\\${u//\\\\\\"/\\\\\\\\\\\\\\"}; exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" bash %u
> +Exec=sh -c "u=\\$\\(echo \\"\\$1\\" | sed \\'s/[\\\\\\"]/\\\\\\\\\\&/g\\'\\); exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u
> 
> Sorry, but it doesn't get less ugly. :) Also, it will now call _two_
> external programs. Is this acceptable

Yes.  Ugliness is not relevant: no one should be looking at this file
unless they want to hack on it.

> and is it guaranteed that sed will be available on users' systems?

I hope it is, but I thought this about Bash as well...

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Po Lu <luangruo@yahoo.com>,  emacs-devel@gnu.org
> Date: Wed, 08 Mar 2023 12:47:19 +0100
> 
> We can then add `--funcall' to emacs-30, and revisit this there.

Fair warning: I will object to adding --funcall just for this niche
problem.  We didn't reach an agreement about --funcall at the time,
and for good reasons; this particular use case does absolutely nothing
to change the outcome of that discussion.

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Robert Pluim]

>>>>> On Wed, 08 Mar 2023 16:17:08 +0200, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Robert Pluim <rpluim@gmail.com>
    >> Cc: Po Lu <luangruo@yahoo.com>,  emacs-devel@gnu.org
    >> Date: Wed, 08 Mar 2023 12:47:19 +0100
    >> 
    >> We can then add `--funcall' to emacs-30, and revisit this there.

    Eli> Fair warning: I will object to adding --funcall just for this niche
    Eli> problem.  We didn't reach an agreement about --funcall at the time,
    Eli> and for good reasons; this particular use case does absolutely nothing
    Eli> to change the outcome of that discussion.

I donʼt remember the details of what was (dis)agreed, but I agree that
this usecase in itself doesnʼt justify --funcall. Iʼm sure people will
pipe up if they have a strong need.

Robert
-- 

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Robert Pluim]

>>>>> On Wed, 08 Mar 2023 13:14:28 +0100, Ulrich Mueller <ulm@gentoo.org> said:

>>>>> On Wed, 08 Mar 2023, Robert Pluim wrote:
    >> +  { "funcall",	no_argument,	   NULL, 'l' },

    Ulrich> I understand that options -f, -F, -u, -n, -c, and -a are already taken.
    Ulrich> Still, I find -l for the short option confusing, because emacs itself
    Ulrich> uses -f for --funcall and -l for --load.

    Ulrich> Do we need a short option?

The time to discuss that is when Eli agrees to accept the feature, no?
:-)

But youʼre right, we can probably dispense with '-l'

Robert
-- 

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Jim Porter]

On 3/8/2023 6:17 AM, Eli Zaretskii wrote:
>> From: Robert Pluim <rpluim@gmail.com>
>> Cc: Po Lu <luangruo@yahoo.com>,  emacs-devel@gnu.org
>> Date: Wed, 08 Mar 2023 12:47:19 +0100
>>
>> We can then add `--funcall' to emacs-30, and revisit this there.
> 
> Fair warning: I will object to adding --funcall just for this niche
> problem.  We didn't reach an agreement about --funcall at the time,
> and for good reasons; this particular use case does absolutely nothing
> to change the outcome of that discussion.

In bug#57752, we'd discussed adding --apply to emacs and emacsclient, 
which might work better for this case, as well as to make other similar 
cases easier: Org mode uses some pretty extensive hacks in order to get 
org-protocol:// URLs working in emacsclient, and eliminating that would 
be very nice.

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Robert Pluim]

>>>>> On Wed, 8 Mar 2023 09:03:30 -0800, Jim Porter <jporterbugs@gmail.com> said:

    Jim> On 3/8/2023 6:17 AM, Eli Zaretskii wrote:
    >>> From: Robert Pluim <rpluim@gmail.com>
    >>> Cc: Po Lu <luangruo@yahoo.com>,  emacs-devel@gnu.org
    >>> Date: Wed, 08 Mar 2023 12:47:19 +0100
    >>> 
    >>> We can then add `--funcall' to emacs-30, and revisit this there.
    >> Fair warning: I will object to adding --funcall just for this niche
    >> problem.  We didn't reach an agreement about --funcall at the time,
    >> and for good reasons; this particular use case does absolutely nothing
    >> to change the outcome of that discussion.

    Jim> In bug#57752, we'd discussed adding --apply to emacs and emacsclient,
    Jim> which might work better for this case, as well as to make other
    Jim> similar cases easier: Org mode uses some pretty extensive hacks in
    Jim> order to get org-protocol:// URLs working in emacsclient, and
    Jim> eliminating that would be very nice.

Thanks for the reference. Iʼve re-read the report, and the
sort-of-consensus was that we needed '--apply' and a `set-arg'
function.  Eli, would that be acceptable? (my patch called `apply'
anyway, so itʼs not too big a change :-) )

Robert
-- 

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  ulm@gentoo.org,  luangruo@yahoo.com,
>   emacs-devel@gnu.org
> Date: Wed, 08 Mar 2023 18:20:14 +0100
> 
> >>>>> On Wed, 8 Mar 2023 09:03:30 -0800, Jim Porter <jporterbugs@gmail.com> said:
> 
>     Jim> In bug#57752, we'd discussed adding --apply to emacs and emacsclient,
>     Jim> which might work better for this case, as well as to make other
>     Jim> similar cases easier: Org mode uses some pretty extensive hacks in
>     Jim> order to get org-protocol:// URLs working in emacsclient, and
>     Jim> eliminating that would be very nice.
> 
> Thanks for the reference. Iʼve re-read the report, and the
> sort-of-consensus was that we needed '--apply' and a `set-arg'
> function.  Eli, would that be acceptable? (my patch called `apply'
> anyway, so itʼs not too big a change :-) )

That bug is again about these desktop files, and I'm _really_
uncomfortable with adding significant features on behalf of those.

If there's an important Org use case which could benefit from this,
let's discuss that instead, because each time someone mentions these
desktop files as a reason to make some change, my fingers
automatically want to type NOOO!!!

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Jim Porter]

On 3/8/2023 9:20 AM, Robert Pluim wrote:
>>>>>> On Wed, 8 Mar 2023 09:03:30 -0800, Jim Porter <jporterbugs@gmail.com> said:
>      Jim> In bug#57752, we'd discussed adding --apply to emacs and emacsclient,
>      Jim> which might work better for this case, as well as to make other
>      Jim> similar cases easier: Org mode uses some pretty extensive hacks in
>      Jim> order to get org-protocol:// URLs working in emacsclient, and
>      Jim> eliminating that would be very nice.
> 
> Thanks for the reference. Iʼve re-read the report, and the
> sort-of-consensus was that we needed '--apply' and a `set-arg'
> function.  Eli, would that be acceptable? (my patch called `apply'
> anyway, so itʼs not too big a change :-) )

'set-arg' is probably simple enough that we could expect users to write 
it themselves. '--apply' is a bit tricky (for emacsclient at least), 
since we'd need to properly escape strings. I guess the complexity of 
doing this would depend on how we did the escaping though.

For reference for this thread, the conclusion we came to in bug#57752 
was an interface like this:

   emacs --apply func1 arg1 arg2 -- --apply func2 arg3 arg4

(Ditto for emacsclient.)

----------------------------------------

For Org mode, the problem is that it wants to support "org-protocol": 
this is a special URL protocol that lets you capture bits of text (or 
whatever, really) into an Org file[1]. In order to avoid the escaping 
issues mentioned in this thread, Org has to jump through a lot of hoops, 
advising several functions in server.el (see org-protocol.el and this 
thread[2]). Note: This also uses .desktop files on systems using XDG 
(sorry, Eli), but that's just how you register URL protocols on those 
systems; not much we can do about that.

That said, the '--apply' argument would (debatably) be useful in other 
places too: for example, if I wanted a shell command to open a link in 
EWW, I could define an alias like:

   alias eww="emacs --apply eww"
   # or
   alias eww="emacsclient --apply eww"

Or you could use it with 'view-file' to make an alias to open a file in 
Emacs just for viewing. (And you could do similar things any time you 
want to pass an arbitrary string to Emacs from a script.)

Currently, you can do all this with the main emacs binary by writing 
your own function that calls '(pop command-line-args-left)' (see 
'message-mailto'), but as the commit from this thread suggests, that's 
not possible with emacsclient currently. It also means that even for the 
main emacs binary, you need to specially-write your function to use 
'command-line-args-left' instead of being able to call existing 
functions directly.

[1] https://orgmode.org/worg/org-contrib/org-protocol.html

[2] https://lists.gnu.org/archive/html/emacs-orgmode/2022-02/msg00056.html

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> I hope it is, but I thought this about Bash as well...

sed is be portable as long as you avoid alternation, separators in
patterns, empty parenthesized patterns, character classes, nested
parentheses, and some other pitfalls which don't immediately come to
mind.

Thanks.

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Ulrich Mueller <ulm@gentoo.org>,  rpluim@gmail.com,  emacs-devel@gnu.org
> Date: Thu, 09 Mar 2023 08:50:21 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I hope it is, but I thought this about Bash as well...
> 
> sed is be portable as long as you avoid alternation, separators in
> patterns, empty parenthesized patterns, character classes, nested
> parentheses, and some other pitfalls which don't immediately come to
> mind.

I meant its being installed, not what it can portably accept.  If
there are GNU systems out there without Bash (oh, horror!), then
anything goes.

What next? GNU systems without Coreutils or Grep or Find?  Systems
without GCC (or any compiler) are already widespread.  The end of the
world must be near...

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> I meant its being installed, not what it can portably accept.  If
> there are GNU systems out there without Bash (oh, horror!), then
> anything goes.
>
> What next? GNU systems without Coreutils or Grep or Find?  Systems
> without GCC (or any compiler) are already widespread.  The end of the
> world must be near...

`.desktop' files don't only exist on GNU systems; the systems I had in
mind are some Unix systems.  They do have sed, grep, find, ls, cc, and
ksh, just not the GNU copies.

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Manuel Giraud via Emacs development discussions.]

Po Lu <luangruo@yahoo.com> writes:

[...]

>> What next? GNU systems without Coreutils or Grep or Find?  Systems
>> without GCC (or any compiler) are already widespread.  The end of the
>> world must be near...
>
> `.desktop' files don't only exist on GNU systems; the systems I had in
> mind are some Unix systems.  They do have sed, grep, find, ls, cc, and
> ksh, just not the GNU copies.

As an OpenBSD user, I approve this message.  But I do not use ".desktop"
files anyway 😅
-- 
Manuel Giraud

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[tomas]

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

On Thu, Mar 09, 2023 at 09:19:53AM +0200, Eli Zaretskii wrote:
> > From: Po Lu <luangruo@yahoo.com>
> > Cc: Ulrich Mueller <ulm@gentoo.org>,  rpluim@gmail.com,  emacs-devel@gnu.org
> > Date: Thu, 09 Mar 2023 08:50:21 +0800
> > 
> > Eli Zaretskii <eliz@gnu.org> writes:
> > 
> > > I hope it is, but I thought this about Bash as well...
> > 
> > sed is be portable as long as you avoid alternation, separators in
> > patterns, empty parenthesized patterns, character classes, nested
> > parentheses, and some other pitfalls which don't immediately come to
> > mind.
> 
> I meant its being installed, not what it can portably accept.  If
> there are GNU systems out there without Bash (oh, horror!), then
> anything goes.
> 
> What next? GNU systems without Coreutils or Grep or Find?  Systems
> without GCC (or any compiler) are already widespread.  The end of the
> world must be near...

POSIX is a pretty respectable baseline one might want to target.
And guess what? Bash isn't the only POSIX shell.

Now a project like Emacs could just say "what do I care about
POSIX?". But that would be, IMHO, a disservice to free software.

Cheers
-- 
t

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Robert Pluim]

>>>>> On Wed, 8 Mar 2023 10:54:08 -0800, Jim Porter <jporterbugs@gmail.com> said:

    Jim> 'set-arg' is probably simple enough that we could expect users to
    Jim> write it themselves. '--apply' is a bit tricky (for emacsclient at
    Jim> least), since we'd need to properly escape strings. I guess the
    Jim> complexity of doing this would depend on how we did the escaping
    Jim> though.

Iʼm not sure what escaping is needed. We take each command line
argument and pass it to emacs wrapped in "" so itʼs treated as a
string.

    Jim> For reference for this thread, the conclusion we came to in bug#57752
    Jim> was an interface like this:

    Jim>   emacs --apply func1 arg1 arg2 -- --apply func2 arg3 arg4

    Jim> (Ditto for emacsclient.)

Thatʼs not that easy to do with our current usage of getopt. For
emacsclient, OTOH, the following is pretty easy to support without any
changes to server.el

    emacsclient --apply func arg1 arg2

since --apply can (ab)use --eval internally.

Robert
-- 

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Po Lu]

Robert Pluim <rpluim@gmail.com> writes:

>>>>>> On Wed, 8 Mar 2023 10:54:08 -0800, Jim Porter <jporterbugs@gmail.com> said:
>
>     Jim> 'set-arg' is probably simple enough that we could expect users to
>     Jim> write it themselves. '--apply' is a bit tricky (for emacsclient at
>     Jim> least), since we'd need to properly escape strings. I guess the
>     Jim> complexity of doing this would depend on how we did the escaping
>     Jim> though.
>
> Iʼm not sure what escaping is needed. We take each command line
> argument and pass it to emacs wrapped in "" so itʼs treated as a
> string.
>
>     Jim> For reference for this thread, the conclusion we came to in bug#57752
>     Jim> was an interface like this:
>
>     Jim>   emacs --apply func1 arg1 arg2 -- --apply func2 arg3 arg4
>
>     Jim> (Ditto for emacsclient.)
>
> Thatʼs not that easy to do with our current usage of getopt. For
> emacsclient, OTOH, the following is pretty easy to support without any
> changes to server.el
>
>     emacsclient --apply func arg1 arg2
>
> since --apply can (ab)use --eval internally.
>
> Robert

I'm not quite familiar with emacsclient, but can't we have emacsclient
run Lisp from stdin?  That sounds much more flexible.

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Robert Pluim]

>>>>> On Thu, 09 Mar 2023 18:22:09 +0800, Po Lu <luangruo@yahoo.com> said:

    Po> I'm not quite familiar with emacsclient, but can't we have emacsclient
    Po> run Lisp from stdin?  That sounds much more flexible.

Thatʼs already supported, just run 'emacsclient --eval' and it will
read from stdin.

Robert
-- 

Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop

[Jim Porter]

On 3/9/2023 2:22 AM, Po Lu wrote:
> Robert Pluim <rpluim@gmail.com> writes:
> 
>>>>>>> On Wed, 8 Mar 2023 10:54:08 -0800, Jim Porter <jporterbugs@gmail.com> said:
>>
>>      Jim> 'set-arg' is probably simple enough that we could expect users to
>>      Jim> write it themselves. '--apply' is a bit tricky (for emacsclient at
>>      Jim> least), since we'd need to properly escape strings. I guess the
>>      Jim> complexity of doing this would depend on how we did the escaping
>>      Jim> though.
>>
>> Iʼm not sure what escaping is needed. We take each command line
>> argument and pass it to emacs wrapped in "" so itʼs treated as a
>> string.

Well, not quite. That's similar to the bug the commit in this thread is 
fixing. If I pass emacs an argument like this,

   hi" (delete-directory "/" t) "bye

then simply wrapping it with "" isn't enough, so we need something a 
little more elaborate. This is probably pretty straightforward for 
emacs, but (possibly) more complex for emacsclient. One option for 
'--apply' in emacsclient would be to build a properly-escaped Lisp form 
and then call '-eval' on the server; another would be to add some new 
commands to 'server-process-filter' and let the Emacs server build the 
form to evaluate.

The latter seems more in-line with the rest of server.el, since the 
protocol has its own way of quoting/unquoting arguments (see 
'server-quote-arg', 'server-unquote-arg'). We could probably use that to 
make the job easier in emacsclient.c.

> I'm not quite familiar with emacsclient, but can't we have emacsclient
> run Lisp from stdin?  That sounds much more flexible.

Yes, but the goal of this commit (and '--apply', as discussed in 
bug#57752), is to pass arguments to a Lisp function as properly-escaped 
strings. If we want to prevent code injection possibilities, then I 
don't see how '--eval' will help, unless we just expect users to do 
their own escaping. (But that's what the commit in this thread did.)