* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop [not found] ` <20230307172816.2D56BC13915@vcs2.savannah.gnu.org> @ 2023-03-08 0:27 ` Po Lu 2023-03-08 2:14 ` Ulrich Mueller 0 siblings, 1 reply; 37+ messages in thread From: Po Lu @ 2023-03-08 0:27 UTC (permalink / raw) To: emacs-devel; +Cc: Ulrich Müller 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. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 0:27 ` emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop Po Lu @ 2023-03-08 2:14 ` Ulrich Mueller 2023-03-08 2:24 ` Po Lu 0 siblings, 1 reply; 37+ messages in thread From: Ulrich Mueller @ 2023-03-08 2:14 UTC (permalink / raw) To: Po Lu; +Cc: emacs-devel >>>>> 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? ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 2:14 ` Ulrich Mueller @ 2023-03-08 2:24 ` Po Lu 2023-03-08 7:15 ` Ulrich Mueller 0 siblings, 1 reply; 37+ messages in thread From: Po Lu @ 2023-03-08 2:24 UTC (permalink / raw) To: Ulrich Mueller; +Cc: emacs-devel 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. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 2:24 ` Po Lu @ 2023-03-08 7:15 ` Ulrich Mueller 2023-03-08 8:09 ` Po Lu 2023-03-08 10:37 ` Robert Pluim 0 siblings, 2 replies; 37+ messages in thread From: Ulrich Mueller @ 2023-03-08 7:15 UTC (permalink / raw) To: Po Lu; +Cc: emacs-devel >>>>> 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. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 7:15 ` Ulrich Mueller @ 2023-03-08 8:09 ` Po Lu 2023-03-08 8:32 ` Ulrich Mueller 2023-03-08 10:37 ` Robert Pluim 1 sibling, 1 reply; 37+ messages in thread From: Po Lu @ 2023-03-08 8:09 UTC (permalink / raw) To: Ulrich Mueller; +Cc: emacs-devel 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? ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 8:09 ` Po Lu @ 2023-03-08 8:32 ` Ulrich Mueller 2023-03-08 10:29 ` Po Lu 0 siblings, 1 reply; 37+ messages in thread From: Ulrich Mueller @ 2023-03-08 8:32 UTC (permalink / raw) To: Po Lu; +Cc: emacs-devel >>>>> 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. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 8:32 ` Ulrich Mueller @ 2023-03-08 10:29 ` Po Lu 2023-03-08 10:39 ` Ulrich Mueller 0 siblings, 1 reply; 37+ messages in thread From: Po Lu @ 2023-03-08 10:29 UTC (permalink / raw) To: Ulrich Mueller; +Cc: emacs-devel 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. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 10:29 ` Po Lu @ 2023-03-08 10:39 ` Ulrich Mueller 2023-03-08 10:44 ` Robert Pluim ` (2 more replies) 0 siblings, 3 replies; 37+ messages in thread From: Ulrich Mueller @ 2023-03-08 10:39 UTC (permalink / raw) To: Po Lu; +Cc: emacs-devel >>>>> 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. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 10:39 ` Ulrich Mueller @ 2023-03-08 10:44 ` Robert Pluim 2023-03-08 11:08 ` Ulrich Mueller 2023-03-08 10:58 ` Po Lu 2023-03-08 14:05 ` Eli Zaretskii 2 siblings, 1 reply; 37+ messages in thread From: Robert Pluim @ 2023-03-08 10:44 UTC (permalink / raw) To: Ulrich Mueller; +Cc: Po Lu, emacs-devel >>>>> 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 -- ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 10:44 ` Robert Pluim @ 2023-03-08 11:08 ` Ulrich Mueller 2023-03-08 11:29 ` Ulrich Mueller ` (2 more replies) 0 siblings, 3 replies; 37+ messages in thread From: Ulrich Mueller @ 2023-03-08 11:08 UTC (permalink / raw) To: Robert Pluim; +Cc: Po Lu, emacs-devel >>>>> 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? ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 11:08 ` Ulrich Mueller @ 2023-03-08 11:29 ` Ulrich Mueller 2023-03-08 11:47 ` Robert Pluim 2023-03-08 11:44 ` Po Lu 2023-03-08 14:14 ` Eli Zaretskii 2 siblings, 1 reply; 37+ messages in thread From: Ulrich Mueller @ 2023-03-08 11:29 UTC (permalink / raw) To: Ulrich Mueller; +Cc: Robert Pluim, Po Lu, emacs-devel >>>>> 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 ^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 11:29 ` Ulrich Mueller @ 2023-03-08 11:47 ` Robert Pluim 2023-03-08 14:17 ` Eli Zaretskii 0 siblings, 1 reply; 37+ messages in thread From: Robert Pluim @ 2023-03-08 11:47 UTC (permalink / raw) To: Ulrich Mueller; +Cc: Po Lu, emacs-devel >>>>> 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 -- ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 11:47 ` Robert Pluim @ 2023-03-08 14:17 ` Eli Zaretskii 2023-03-08 15:47 ` Robert Pluim 2023-03-08 17:03 ` Jim Porter 0 siblings, 2 replies; 37+ messages in thread From: Eli Zaretskii @ 2023-03-08 14:17 UTC (permalink / raw) To: Robert Pluim; +Cc: ulm, luangruo, emacs-devel > 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. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 14:17 ` Eli Zaretskii @ 2023-03-08 15:47 ` Robert Pluim 2023-03-08 17:03 ` Jim Porter 1 sibling, 0 replies; 37+ messages in thread From: Robert Pluim @ 2023-03-08 15:47 UTC (permalink / raw) To: Eli Zaretskii; +Cc: ulm, luangruo, emacs-devel >>>>> 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 -- ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 14:17 ` Eli Zaretskii 2023-03-08 15:47 ` Robert Pluim @ 2023-03-08 17:03 ` Jim Porter 2023-03-08 17:20 ` Robert Pluim 1 sibling, 1 reply; 37+ messages in thread From: Jim Porter @ 2023-03-08 17:03 UTC (permalink / raw) To: Eli Zaretskii, Robert Pluim; +Cc: ulm, luangruo, emacs-devel 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. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 17:03 ` Jim Porter @ 2023-03-08 17:20 ` Robert Pluim 2023-03-08 17:41 ` Eli Zaretskii 2023-03-08 18:54 ` Jim Porter 0 siblings, 2 replies; 37+ messages in thread From: Robert Pluim @ 2023-03-08 17:20 UTC (permalink / raw) To: Jim Porter; +Cc: Eli Zaretskii, ulm, luangruo, emacs-devel >>>>> 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 -- ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 17:20 ` Robert Pluim @ 2023-03-08 17:41 ` Eli Zaretskii 2023-03-08 18:54 ` Jim Porter 1 sibling, 0 replies; 37+ messages in thread From: Eli Zaretskii @ 2023-03-08 17:41 UTC (permalink / raw) To: Robert Pluim; +Cc: jporterbugs, ulm, luangruo, emacs-devel > 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!!! ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 17:20 ` Robert Pluim 2023-03-08 17:41 ` Eli Zaretskii @ 2023-03-08 18:54 ` Jim Porter 2023-03-09 9:30 ` Robert Pluim 1 sibling, 1 reply; 37+ messages in thread From: Jim Porter @ 2023-03-08 18:54 UTC (permalink / raw) To: Robert Pluim; +Cc: Eli Zaretskii, ulm, luangruo, emacs-devel 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 ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 18:54 ` Jim Porter @ 2023-03-09 9:30 ` Robert Pluim 2023-03-09 10:22 ` Po Lu 0 siblings, 1 reply; 37+ messages in thread From: Robert Pluim @ 2023-03-09 9:30 UTC (permalink / raw) To: Jim Porter; +Cc: Eli Zaretskii, ulm, luangruo, emacs-devel >>>>> 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 -- ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-09 9:30 ` Robert Pluim @ 2023-03-09 10:22 ` Po Lu 2023-03-09 10:50 ` Robert Pluim 2023-03-09 18:36 ` Jim Porter 0 siblings, 2 replies; 37+ messages in thread From: Po Lu @ 2023-03-09 10:22 UTC (permalink / raw) To: Robert Pluim; +Cc: Jim Porter, Eli Zaretskii, ulm, emacs-devel 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. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-09 10:22 ` Po Lu @ 2023-03-09 10:50 ` Robert Pluim 2023-03-09 18:36 ` Jim Porter 1 sibling, 0 replies; 37+ messages in thread From: Robert Pluim @ 2023-03-09 10:50 UTC (permalink / raw) To: Po Lu; +Cc: Jim Porter, Eli Zaretskii, ulm, emacs-devel >>>>> 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 -- ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-09 10:22 ` Po Lu 2023-03-09 10:50 ` Robert Pluim @ 2023-03-09 18:36 ` Jim Porter 1 sibling, 0 replies; 37+ messages in thread From: Jim Porter @ 2023-03-09 18:36 UTC (permalink / raw) To: Po Lu, Robert Pluim; +Cc: Eli Zaretskii, ulm, emacs-devel 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.) ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 11:08 ` Ulrich Mueller 2023-03-08 11:29 ` Ulrich Mueller @ 2023-03-08 11:44 ` Po Lu 2023-03-08 14:14 ` Eli Zaretskii 2 siblings, 0 replies; 37+ messages in thread From: Po Lu @ 2023-03-08 11:44 UTC (permalink / raw) To: Ulrich Mueller; +Cc: Robert Pluim, emacs-devel 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. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 11:08 ` Ulrich Mueller 2023-03-08 11:29 ` Ulrich Mueller 2023-03-08 11:44 ` Po Lu @ 2023-03-08 14:14 ` Eli Zaretskii 2023-03-09 0:50 ` Po Lu 2 siblings, 1 reply; 37+ messages in thread From: Eli Zaretskii @ 2023-03-08 14:14 UTC (permalink / raw) To: Ulrich Mueller; +Cc: rpluim, luangruo, emacs-devel > 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... ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 14:14 ` Eli Zaretskii @ 2023-03-09 0:50 ` Po Lu 2023-03-09 7:19 ` Eli Zaretskii 0 siblings, 1 reply; 37+ messages in thread From: Po Lu @ 2023-03-09 0:50 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Ulrich Mueller, rpluim, emacs-devel 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. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-09 0:50 ` Po Lu @ 2023-03-09 7:19 ` Eli Zaretskii 2023-03-09 7:25 ` Po Lu 2023-03-09 8:20 ` tomas 0 siblings, 2 replies; 37+ messages in thread From: Eli Zaretskii @ 2023-03-09 7:19 UTC (permalink / raw) To: Po Lu; +Cc: ulm, rpluim, emacs-devel > 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... ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-09 7:19 ` Eli Zaretskii @ 2023-03-09 7:25 ` Po Lu 2023-03-09 7:49 ` Manuel Giraud via Emacs development discussions. 2023-03-09 8:20 ` tomas 1 sibling, 1 reply; 37+ messages in thread From: Po Lu @ 2023-03-09 7:25 UTC (permalink / raw) To: Eli Zaretskii; +Cc: ulm, rpluim, emacs-devel 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. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-09 7:25 ` Po Lu @ 2023-03-09 7:49 ` Manuel Giraud via Emacs development discussions. 0 siblings, 0 replies; 37+ messages in thread From: Manuel Giraud via Emacs development discussions. @ 2023-03-09 7:49 UTC (permalink / raw) To: Po Lu; +Cc: Eli Zaretskii, ulm, rpluim, emacs-devel 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 ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-09 7:19 ` Eli Zaretskii 2023-03-09 7:25 ` Po Lu @ 2023-03-09 8:20 ` tomas 1 sibling, 0 replies; 37+ messages in thread From: tomas @ 2023-03-09 8:20 UTC (permalink / raw) To: emacs-devel [-- 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 --] ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 10:39 ` Ulrich Mueller 2023-03-08 10:44 ` Robert Pluim @ 2023-03-08 10:58 ` Po Lu 2023-03-08 11:44 ` Ulrich Mueller 2023-03-08 14:13 ` Eli Zaretskii 2023-03-08 14:05 ` Eli Zaretskii 2 siblings, 2 replies; 37+ messages in thread From: Po Lu @ 2023-03-08 10:58 UTC (permalink / raw) To: Ulrich Mueller; +Cc: emacs-devel 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. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 10:58 ` Po Lu @ 2023-03-08 11:44 ` Ulrich Mueller 2023-03-08 14:13 ` Eli Zaretskii 1 sibling, 0 replies; 37+ messages in thread From: Ulrich Mueller @ 2023-03-08 11:44 UTC (permalink / raw) To: Po Lu; +Cc: emacs-devel >>>>> 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. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 10:58 ` Po Lu 2023-03-08 11:44 ` Ulrich Mueller @ 2023-03-08 14:13 ` Eli Zaretskii 1 sibling, 0 replies; 37+ messages in thread From: Eli Zaretskii @ 2023-03-08 14:13 UTC (permalink / raw) To: Po Lu; +Cc: ulm, emacs-devel > 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. ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 10:39 ` Ulrich Mueller 2023-03-08 10:44 ` Robert Pluim 2023-03-08 10:58 ` Po Lu @ 2023-03-08 14:05 ` Eli Zaretskii 2 siblings, 0 replies; 37+ messages in thread From: Eli Zaretskii @ 2023-03-08 14:05 UTC (permalink / raw) To: Ulrich Mueller; +Cc: luangruo, emacs-devel > 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.) ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 7:15 ` Ulrich Mueller 2023-03-08 8:09 ` Po Lu @ 2023-03-08 10:37 ` Robert Pluim 2023-03-08 12:14 ` Ulrich Mueller 2023-03-08 14:04 ` Eli Zaretskii 1 sibling, 2 replies; 37+ messages in thread From: Robert Pluim @ 2023-03-08 10:37 UTC (permalink / raw) To: Ulrich Mueller; +Cc: Po Lu, emacs-devel >>>>> 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 ^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 10:37 ` Robert Pluim @ 2023-03-08 12:14 ` Ulrich Mueller 2023-03-08 15:49 ` Robert Pluim 2023-03-08 14:04 ` Eli Zaretskii 1 sibling, 1 reply; 37+ messages in thread From: Ulrich Mueller @ 2023-03-08 12:14 UTC (permalink / raw) To: Robert Pluim; +Cc: Po Lu, emacs-devel >>>>> 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? ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 12:14 ` Ulrich Mueller @ 2023-03-08 15:49 ` Robert Pluim 0 siblings, 0 replies; 37+ messages in thread From: Robert Pluim @ 2023-03-08 15:49 UTC (permalink / raw) To: Ulrich Mueller; +Cc: Po Lu, emacs-devel >>>>> 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 -- ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop 2023-03-08 10:37 ` Robert Pluim 2023-03-08 12:14 ` Ulrich Mueller @ 2023-03-08 14:04 ` Eli Zaretskii 1 sibling, 0 replies; 37+ messages in thread From: Eli Zaretskii @ 2023-03-08 14:04 UTC (permalink / raw) To: Robert Pluim; +Cc: ulm, luangruo, emacs-devel > 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. ^ permalink raw reply [flat|nested] 37+ messages in thread
end of thread, other threads:[~2023-03-09 18:36 UTC | newest] Thread overview: 37+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <167821009581.14664.5608674978571454819@vcs2.savannah.gnu.org> [not found] ` <20230307172816.2D56BC13915@vcs2.savannah.gnu.org> 2023-03-08 0:27 ` emacs-29 3c1693d08b0: Fix Elisp code injection vulnerability in emacsclient-mail.desktop Po Lu 2023-03-08 2:14 ` Ulrich Mueller 2023-03-08 2:24 ` Po Lu 2023-03-08 7:15 ` Ulrich Mueller 2023-03-08 8:09 ` Po Lu 2023-03-08 8:32 ` Ulrich Mueller 2023-03-08 10:29 ` Po Lu 2023-03-08 10:39 ` Ulrich Mueller 2023-03-08 10:44 ` Robert Pluim 2023-03-08 11:08 ` Ulrich Mueller 2023-03-08 11:29 ` Ulrich Mueller 2023-03-08 11:47 ` Robert Pluim 2023-03-08 14:17 ` Eli Zaretskii 2023-03-08 15:47 ` Robert Pluim 2023-03-08 17:03 ` Jim Porter 2023-03-08 17:20 ` Robert Pluim 2023-03-08 17:41 ` Eli Zaretskii 2023-03-08 18:54 ` Jim Porter 2023-03-09 9:30 ` Robert Pluim 2023-03-09 10:22 ` Po Lu 2023-03-09 10:50 ` Robert Pluim 2023-03-09 18:36 ` Jim Porter 2023-03-08 11:44 ` Po Lu 2023-03-08 14:14 ` Eli Zaretskii 2023-03-09 0:50 ` Po Lu 2023-03-09 7:19 ` Eli Zaretskii 2023-03-09 7:25 ` Po Lu 2023-03-09 7:49 ` Manuel Giraud via Emacs development discussions. 2023-03-09 8:20 ` tomas 2023-03-08 10:58 ` Po Lu 2023-03-08 11:44 ` Ulrich Mueller 2023-03-08 14:13 ` Eli Zaretskii 2023-03-08 14:05 ` Eli Zaretskii 2023-03-08 10:37 ` Robert Pluim 2023-03-08 12:14 ` Ulrich Mueller 2023-03-08 15:49 ` Robert Pluim 2023-03-08 14:04 ` Eli Zaretskii
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).