emacs-orgmode
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[SECURITY] Arbitrary code evaluation security in Org (was: [PATCH] ob-co


From: Ihor Radchenko
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

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.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=345402148

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' +
   `org-link-elisp-confirm-function'.

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

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/safe/prompt/function)
                 (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.

WDYT?

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]