[BUG][Security] begin_src :var evaluated before the prompt to confirm execution

[BUG][Security] begin_src :var evaluated before the prompt to confirm execution

[Max Nikulin]

Hi,

At first I am apologizing. I believed that a dedicated report raising 
this issue was posted to this mailing list by somebody. I can not find 
such message and in my notes the heading is linked to a quite general 
discussion related to source blocks.

Consider the following source block

---- >8 ----
#+begin_src elisp :var a=(message "%s" "pwnd")
   a
#+end_src
---- 8< ----

Open the "*Messages*" buffer (C-h e) and try to evaluate the source 
block (C-c C-c).

Actual result:
"pwnd" message appears in "*Messages*" simultaneously with user prompt 
whether the code should be executed.

Expected result:
No code from the Org buffer and linked files is executed prior to 
confirmation from the user.

Emacs-26.3, Org version is current main HEAD:

6bbd08f5a 2022-10-26 15:15:42 +0800 Ihor Radchenko: 
org-datetree-insert-line: Fix blank line insertion

I consider such issues as a reason why it is bad idea to use Emacs as a 
handler for Org files downloaded from web. Such files should be 
inspected in some viewer unable to execute embedded code at first. A 
strong reason should be necessary to call Emacs for a file from 
non-trusted source.

I never considered this issue as a really urgent one because a user 
should at least hit C-c C-c to activate malicious code. It has similar 
severity as refreshing table cell formulas that would be almost unusable 
if protected by user prompt.

To be honest, this is the only real issue I have noticed since people on 
this list tried to convince me 2 years ago that Org is quite safe in 
respect to unsolicited execution of embedded code.

Re: [BUG][Security] begin_src :var evaluated before the prompt to confirm execution

[Jean Louis]

* Max Nikulin <manikulin@gmail.com> [2022-10-27 06:21]:
> Expected result:
> No code from the Org buffer and linked files is executed prior to
> confirmation from the user.

Should that be or is it a general policy for Org mode?

> Emacs-26.3, Org version is current main HEAD:
> 
> 6bbd08f5a 2022-10-26 15:15:42 +0800 Ihor Radchenko:
> org-datetree-insert-line: Fix blank line insertion
> 
> I consider such issues as a reason why it is bad idea to use Emacs as a
> handler for Org files downloaded from web. 

I am lost here. Should people then use Vim as handler for Org files? 👀

In general, browsers use text/html and other content types they
consider safe, while they let to users decide how to handle various
content types.

The core of the problem is that MIME type such as text/x-org shall be
text and nothing else.

It is up to Org developers to understand MIME types and content types.

It is IMHO obligatory for Org developers NOT to advise people using
incorrect MIME type "text/x-org" -- because Org file by default is not
just text, it is combination of application processing data and text.

Here some references:

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Type
https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types
https://datatracker.ietf.org/doc/html/rfc6838#section-4.2.5

Now what we have at hand is not official MIME type: "text/x-org" which
is by default opened by Emacs text editor, and Emacs does not treat it
exclusively as text, but as combination of application and text.

And that is wrong.

Now is up to developers to understand that combined condition and to
help Emacs to get two distinct Org treatments:

- Org mode as MIME type (Content Type) "text/x-org" which NEVER
  executes any code, but treats Org mode exclusively as text; by
  teaching Emacs not to run or process code in such Org files with
  such content type;

between the condition:

- Org mode as MIME type "application/x-org" which has inside of Org file
  executable functions which Emacs would execute by opening it;

One can provide regular expressions to tools like `file' to recognize
if the Org file is "application/x-org" or just "text/x-org" (remove x-
when MIME type becomes official).

For reference, please see:
https://datatracker.ietf.org/doc/html/rfc6838#section-4.2.5

Or otherwise -- if the above job cannot be done, then please stop
advertising the content type "text/x-org" -- because Org is not text
in the context of RFC6838:
https://datatracker.ietf.org/doc/html/rfc6838

> Such files should be inspected in some viewer unable to execute
> embedded code at first. A strong reason should be necessary to call
> Emacs for a file from non-trusted source.

See above. It is up to Org developers to decide if Org file is
"application" in MIME sense, or if they wish to keep it truly "text"
in MIME sense or if they wish to make different mode for viewing Org
files and allowing Org files to be application

> To be honest, this is the only real issue I have noticed since
> people on this list tried to convince me 2 years ago that Org is
> quite safe in respect to unsolicited execution of embedded code.

I hope that people will understand that Org is "application" and not
just "plain text" as advertised.

And thanks for pointing it out as it helps in resolving confusions.

-- 
Jean

Take action in Free Software Foundation campaigns:
https://www.fsf.org/campaigns

In support of Richard M. Stallman
https://stallmansupport.org/

Re: [BUG][Security] begin_src :var evaluated before the prompt to confirm execution

[Max Nikulin]

On 27/10/2022 11:22, Jean Louis wrote:
> * Max Nikulin [2022-10-27 06:21]:
>> Expected result:
>> No code from the Org buffer and linked files is executed prior to
>> confirmation from the user.
> 
> Should that be or is it a general policy for Org mode?

I am afraid, it is unrealistic. Spreadsheet feature will be unusable. 
And it is another reason why I am strongly against formats designed for 
personal use rather than for web in browser context.

>> I consider such issues as a reason why it is bad idea to use Emacs as a
>> handler for Org files downloaded from web.
> 
> I am lost here. Should people then use Vim as handler for Org files? 👀

I will respond if you ask the same in another thread. I created this one 
to track particular issue: arguments of source code blocks evaluated 
without a prompt. Content-Type will not help fix this issue.

Perhaps closely related issues exist with noweb references or some other 
way to use code outside of particular #+begin_src body. I am unsure if 
such problems should be discussed in this thread or in dedicated ones. I 
am still suffering from deja vu and I should just bump an earlier thread.

Re: [BUG][Security] begin_src :var evaluated before the prompt to confirm execution

[Ihor Radchenko]

Max Nikulin <manikulin@gmail.com> writes:

>> Should that be or is it a general policy for Org mode?
>
> I am afraid, it is unrealistic. Spreadsheet feature will be unusable. 
> And it is another reason why I am strongly against formats designed for 
> personal use rather than for web in browser context.

I disagree. We may ask about execution once for all the spreadsheet
formulas. Could you create an example of a spreadsheet running elisp
code and post it?

> Perhaps closely related issues exist with noweb references or some other 
> way to use code outside of particular #+begin_src body. I am unsure if 
> such problems should be discussed in this thread or in dedicated ones. I 
> am still suffering from deja vu and I should just bump an earlier thread.

It is better to open a new thread compared to none.
If something is briefly discussed in an old thread, it is unlikely to be
noticed. Especially bugs not listed on updates.orgmode.org.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [BUG][Security] begin_src :var evaluated before the prompt to confirm execution

[Ihor Radchenko]

Jean Louis <bugs@gnu.support> writes:

> * Max Nikulin <manikulin@gmail.com> [2022-10-27 06:21]:
>> Expected result:
>> No code from the Org buffer and linked files is executed prior to
>> confirmation from the user.
>
> Should that be or is it a general policy for Org mode?

Yes, it is a general policy.
Org should not execute arbitrary Elisp without confirmation, unless the
user customizes the confirmation query to non-default.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [BUG][Security] begin_src :var evaluated before the prompt to confirm execution

[Max Nikulin]

On 28/10/2022 10:19, Ihor Radchenko wrote:
> Jean Louis writes:
> 
>> * Max Nikulin [2022-10-27 06:21]:
>>> Expected result:
>>> No code from the Org buffer and linked files is executed prior to
>>> confirmation from the user.
>>
>> Should that be or is it a general policy for Org mode?
> 
> Yes, it is a general policy.
> Org should not execute arbitrary Elisp without confirmation, unless the
> user customizes the confirmation query to non-default.

There are significantly different contexts: trusted files created 
locally and arbitrary files fetched through some link in the web. 
Features really convenient in the former case may became a disaster in 
the latter.

If a user is prompted to confirm evaluation of each table formula then 
spreadsheet feature becomes unusable.

---- >8 ----
Enter value and press =TAB=
|   | Value | Result |
|---+-------+--------|
| # |       |        |
#+tblfm: $3='(progn (message "%s" "pwnd") 0)
---- 8< ----

I suspect a bunch of similar problems with export feature. The ability 
to save an .org file as a nicely formatted PDF is great but 
simultaneously dangerous for files obtained from the net. I would like 
to have safe export, but I am afraid that actually the code would be 
fragile.

Re: [BUG][Security] begin_src :var evaluated before the prompt to confirm execution

[Jean Louis]

* Ihor Radchenko <yantar92@posteo.net> [2022-10-28 06:19]:
> Jean Louis <bugs@gnu.support> writes:
> 
> > * Max Nikulin <manikulin@gmail.com> [2022-10-27 06:21]:
> >> Expected result:
> >> No code from the Org buffer and linked files is executed prior to
> >> confirmation from the user.
> >
> > Should that be or is it a general policy for Org mode?
> 
> Yes, it is a general policy.
> Org should not execute arbitrary Elisp without confirmation, unless the
> user customizes the confirmation query to non-default.

✔️ That is nice to know. It opens doors for browsing Org files within Emacs.

-- 
Jean

Take action in Free Software Foundation campaigns:
https://www.fsf.org/campaigns

In support of Richard M. Stallman
https://stallmansupport.org/

[PATCH] Re: [BUG][Security] begin_src :var evaluated before the prompt to confirm execution

[Ihor Radchenko]

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

Max Nikulin <manikulin@gmail.com> writes:

> Consider the following source block
>
> ---- >8 ----
> #+begin_src elisp :var a=(message "%s" "pwnd")
>    a
> #+end_src
> ---- 8< ----
>
> Open the "*Messages*" buffer (C-h e) and try to evaluate the source 
> block (C-c C-c).
>
> Actual result:
> "pwnd" message appears in "*Messages*" simultaneously with user prompt 
> whether the code should be executed.
>
> Expected result:
> No code from the Org buffer and linked files is executed prior to 
> confirmation from the user.

Confirmed.

See the attached tentative patch.
I tried to balance between annoying users with query and not evaluating
unsafe code: '-quoted lists and symbols are still evaluated without
prompt.

Let me know if you see any potential issues.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-babel-read-Obey-org-confirm-babel-evaluate.patch --]
[-- Type: text/x-patch, Size: 1683 bytes --]

From 961a3ee38a2b9521427fddea5102d003f39b88d6 Mon Sep 17 00:00:00 2001
Message-Id: <961a3ee38a2b9521427fddea5102d003f39b88d6.1666926796.git.yantar92@posteo.net>
From: Ihor Radchenko <yantar92@posteo.net>
Date: Fri, 28 Oct 2022 11:09:50 +0800
Subject: [PATCH] org-babel-read: Obey `org-confirm-babel-evaluate'

* lisp/ob-core.el (org-babel-read): Do not execute arbitrary Elisp
without prompt, according to `org-confirm-babel-evaluate'.

Reported-by: Max Nikulin <manikulin@gmail.com>
Link: https://orgmode.org/list/tjct9e$179u$1@ciao.gmane.io
---
 lisp/ob-core.el | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 518831ec6..e10ab401c 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -3165,7 +3165,16 @@ (defun org-babel-read (cell &optional inhibit-lisp-eval)
 	((and (not inhibit-lisp-eval)
 	      (or (memq (string-to-char cell) '(?\( ?' ?` ?\[))
 		  (string= cell "*this*")))
-	 (eval (read cell) t))
+         ;; Prevent arbitrary function calls.
+         (if (and (memq (string-to-char cell) '(?\( ?`))
+                  (not (org-babel-confirm-evaluate
+                      ;; See `org-babel-get-src-block-info'.
+                      (list "emacs-lisp" (format "%S" cell)
+                            '((:eval . yes)) nil (format "%S" cell)
+                            nil nil))))
+             ;; Not allowed.
+             (user-error "Evaluation of elisp code %S aborted." cell)
+	   (eval (read cell) t)))
 	((save-match-data
            (and (string-match "^[[:space:]]*\"\\(.*\\)\"[[:space:]]*$" cell)
                 (not (string-match "[^\\]\"" (match-string 1 cell)))))
-- 
2.35.1


[-- Attachment #3: Type: text/plain, Size: 225 bytes --]



-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] Re: [BUG][Security] begin_src :var evaluated before the prompt to confirm execution

[Max Nikulin]

On 28/10/2022 10:15, Ihor Radchenko wrote:
> 
> See the attached tentative patch.
> I tried to balance between annoying users with query and not evaluating
> unsafe code: '-quoted lists and symbols are still evaluated without
> prompt.
> 
> Let me know if you see any potential issues.

If I got it right, it prompt user for every variable. I believed that 
single prompt is enough for both header arguments and body evaluation. 
Maybe I missed some issue with dependent code blocks. If I remember 
correctly, each block causes a prompt and I am not going to dispute such 
behavior. Unsure if early prompt may increase user confusion since some 
blocks will be evaluated not immediately after related prompt but 
perhaps after some delay to confirm dependent code.

> diff --git a/lisp/ob-core.el b/lisp/ob-core.el
> index 518831ec6..e10ab401c 100644
> --- a/lisp/ob-core.el
> +++ b/lisp/ob-core.el
> @@ -3165,7 +3165,16 @@ (defun org-babel-read (cell &optional inhibit-lisp-eval)
>  	((and (not inhibit-lisp-eval)
>  	      (or (memq (string-to-char cell) '(?\( ?' ?` ?\[))
>  		  (string= cell "*this*")))
> -	 (eval (read cell) t))
> +         ;; Prevent arbitrary function calls.
> +         (if (and (memq (string-to-char cell) '(?\( ?`))
> +                  (not (org-babel-confirm-evaluate
> +                      ;; See `org-babel-get-src-block-info'.
> +                      (list "emacs-lisp" (format "%S" cell)
> +                            '((:eval . yes)) nil (format "%S" cell)
> +                            nil nil))))
> +             ;; Not allowed.
> +             (user-error "Evaluation of elisp code %S aborted." cell)
> +	   (eval (read cell) t)))
>  	((save-match-data
>             (and (string-match "^[[:space:]]*\"\\(.*\\)\"[[:space:]]*$" cell)
>                  (not (string-match "[^\\]\"" (match-string 1 cell)))))

Re: [PATCH] Re: [BUG][Security] begin_src :var evaluated before the prompt to confirm execution

[Ihor Radchenko]

Max Nikulin <manikulin@gmail.com> writes:

> On 28/10/2022 10:15, Ihor Radchenko wrote:
>> 
>> See the attached tentative patch.
>> I tried to balance between annoying users with query and not evaluating
>> unsafe code: '-quoted lists and symbols are still evaluated without
>> prompt.
>> 
>> Let me know if you see any potential issues.
>
> If I got it right, it prompt user for every variable. I believed that 
> single prompt is enough for both header arguments and body evaluation. 
> Maybe I missed some issue with dependent code blocks. If I remember 
> correctly, each block causes a prompt and I am not going to dispute such 
> behavior. Unsure if early prompt may increase user confusion since some 
> blocks will be evaluated not immediately after related prompt but 
> perhaps after some delay to confirm dependent code.

You are right that a separate prompt will be shown for each eval form,
even when multiple :var assignments are present in the header arguments.

I could try to patch the higher-level function that is causing the
"pwned" in your example, but I figured that org-babel-read is actually
used in many more places and thus similar issue could occur in different
scenarios. Displaying prompt in org-babel-read is the safest approach I
managed to come up with.

What can help here is the feature request about bulk-yes/bulk-no code
prompts:
https://list.orgmode.org/orgmode/l-mfSe5KD9g7LPdD7qHPtqtjBD45DK8OeOqO6ybMWqoXG-oEQAFjlW1V1N1NtJQBWM_A4CeGT_92rpSKF7Aq0KRzsNmMZfAJnAbyVWeCjoA=@protonmail.ch/

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] Re: [BUG][Security] begin_src :var evaluated before the prompt to confirm execution

[Ihor Radchenko]

Ihor Radchenko <yantar92@posteo.net> writes:

> See the attached tentative patch.
> I tried to balance between annoying users with query and not evaluating
> unsafe code: '-quoted lists and symbols are still evaluated without
> prompt.

Fixed.
Applied onto main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=10e857d42859a55b23cd4206ffce3ebd0f678583

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>