emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Ihor Radchenko <yantar92@posteo.net>
To: Tom Gillespie <tgbugs@gmail.com>
Cc: Bastien <bzg@gnu.org>, Kyle Meyer <kyle@kyleam.com>,
Subject: [SECURITY] Arbitrary code evaluation security in Org (was: [PATCH] ob-core: add org-confirm-babel-evaluate-cell custom variable)
Date: Mon, 02 Jan 2023 08:34:30 +0000	[thread overview]
Message-ID: <87edsd5o89.fsf@localhost> (raw)
In-Reply-To: <878rinadlq.fsf@localhost>

Ihor Radchenko <yantar92@posteo.net> writes:

> P.S. Considering intense discussion around the topic, what about
> reverting my commit from the release? We can then re-consider the whole
> design and apply something more elaborate later.

I now reverted the discussed commit.

We need to come up with a uniform security system where all the code
evaluation is (1) easy to control via variables and user queries; (2)
not too annoying for users; (3) provides the necessary context for users
to decide if a code is safe to run.

Before we continue the discussion, I will try to summarize the current
state of Org manual and code wrt code evaluation of the code coming from
Org documents (including downloaded Org files).

In the manual we now have
17.13 Code Evaluation and Security Issues

This section talks about

1. Source block bodies and `org-confirm-babel-evaluate'
2. shell/elisp links and `org-link-shell-confirm-function' +

   Notably, `org-link-elisp-skip-confirm-regexp' is not mentioned in the

3. Table cells, with no way to query or disable execution.

In reality, we have many more places in the code where arbitrary text
from Org files can be evaluated.

I have marked some places in the above commit with FIXME.

From my audit of Org sources the following scenarios may lead to
arbitrary code execution:

1. Code block bodies
2. Elisp sexps in header args. Notably, not only `org-babel-read' is
   responsible for evaluating header args. :results and :exports are
   evaluated independently.
3. Table cells
4. "eval" macros during expansion
5. Diary sexps
To the best of my knowledge, this list should be complete. Though I
would appreciate someone double-checking.

According to the above:

- `org-confirm-babel-evaluate' allows either using
  `org-babel-confirm-evaluate' prompt (when t), not asking at all (when
  nil), or using arbitrary prompt function.
- `org-link-shell-confirm-function' + `org-link-elisp-confirm-function'
  are similar, except defaulting to `yes-or-no-p'.
- `org-link-elisp-skip-confirm-regexp' extends indiscriminate function
  queries to also skip certain (single) regexp.

The situation is not ideal.

We have similar, but more detailed system for downloading remote files:

- `org-safe-remote-resources' allows users to define certain files/urls
  that are known to be safe
- `org-resource-download-policy' provides choice between "always query",
  "query unless safe", "never download", and "always download"

Also, `org--confirm-resource-safe', unlike `org-babel-confirm-evaluate'
allows manipulating `org-safe-remote-resources' interactively by marking
current URL or URL domain safe for future.

What we can do

1. Introduce a new customization `org-confirm-evaluate-safe-regexps'
   listing regexps that are considered safe or cons cells
   (src-body/header-arg/table/macro/diary . regexp)

2. Introduce a new customization `org-confirm-evaluate' that can be set
   to t/nil/prompt/safe/[function]/[alist]:
   - t/safe :: to display default prompt unless the code block in buffer is
               marked safe
   - prompt :: always ask (the prompt will then not display options to
               add current sexp to safe list)
   - [function] :: custom function that returns t/safe/prompt/[alist]
   - [alist] :: (src-body/header-arg/table/macro/diary/t .
                 (t . <value>) stands for default value.

3. The default prompt will mimic `org--confirm-resource-safe',
   additionally accepting Y/N to accept/decline all the evaluation in
   current command.

This system will be used across Org for code evaluation. Old variables
will be supported, but obsoleted.


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>

  parent reply	other threads:[~2023-01-02  8:35 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-10 20:28 [PATCH] ob-core: add org-confirm-babel-evaluate-cell custom variable Tom Gillespie
2022-12-11  2:58 ` Max Nikulin
2022-12-11 20:27   ` Tom Gillespie
2022-12-11 20:37     ` Tom Gillespie
2022-12-11 20:46     ` Kyle Meyer
2022-12-11 21:08       ` Tom Gillespie
2022-12-12 10:20         ` Ihor Radchenko
2022-12-13  1:53           ` Tom Gillespie
2022-12-13  9:03             ` Ihor Radchenko
2022-12-13 16:31             ` Max Nikulin
2022-12-13 21:16               ` Tom Gillespie
2022-12-14 16:40                 ` Max Nikulin
2022-12-14 18:24                   ` Tom Gillespie
2022-12-15  9:18                     ` Ihor Radchenko
2022-12-15  9:25                       ` Tom Gillespie
2022-12-15  9:57                       ` tomas
2022-12-15  9:10                   ` Ihor Radchenko
2022-12-15 12:10                     ` Max Nikulin
2022-12-15 12:25                       ` Ihor Radchenko
2022-12-15 14:46                         ` Max Nikulin
2022-12-15 21:08                           ` Tim Cross
2022-12-16  6:07                             ` Ihor Radchenko
2022-12-16  7:22                               ` Tim Cross
2022-12-18 14:19                                 ` Ihor Radchenko
2022-12-18 21:37                                   ` Tim Cross
2022-12-20  0:00                                     ` Tom Gillespie
2022-12-20  0:06                                       ` Tom Gillespie
2022-12-25 11:00                                         ` Ihor Radchenko
2022-12-18 14:12                           ` Ihor Radchenko
2022-12-25 11:06             ` Ihor Radchenko
2022-12-29 15:58               ` Bastien Guerry
2022-12-29 16:33                 ` Max Nikulin
2022-12-29 16:35                 ` Ihor Radchenko
2022-12-30  8:52                   ` Bastien
2022-12-30 11:10                     ` Max Nikulin
2022-12-30 17:43                     ` Tom Gillespie
2022-12-31 13:48                       ` Ihor Radchenko
2022-12-31 16:15                         ` Tom Gillespie
2023-01-02  8:34                         ` Ihor Radchenko [this message]
2023-01-02 10:59                           ` [SECURITY] Arbitrary code evaluation security in Org Greg Minshall
2023-01-03  9:52                             ` [SECURITY] Tangling can overwrite arbitrary tangling targets, including important user files (was: [SECURITY] Arbitrary code evaluation security in Org) Ihor Radchenko
2023-01-02 19:00                           ` [SECURITY] Arbitrary code evaluation security in Org (was: [PATCH] ob-core: add org-confirm-babel-evaluate-cell custom variable) Tim Cross
2023-01-03 11:00                             ` Ihor Radchenko
2023-01-07 13:12                               ` Ihor Radchenko
2023-01-02 15:13                         ` [PATCH] ob-core: add org-confirm-babel-evaluate-cell custom variable Bastien Guerry
2023-01-02 15:17                           ` Ihor Radchenko
2023-01-02 15:15                       ` Bastien
2022-12-13  4:16           ` Kyle Meyer
2022-12-13 16:15     ` Max Nikulin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87edsd5o89.fsf@localhost \
    --to=yantar92@posteo.net \
    --cc=bzg@gnu.org \
    --cc=emacs-orgmode@gnu.org \
    --cc=kyle@kyleam.com \
    --cc=tgbugs@gmail.com \


* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox


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).