emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Tom Gillespie <tgbugs@gmail.com>
To: Max Nikulin <manikulin@gmail.com>
Cc: emacs-orgmode@gnu.org
Subject: Re: [PATCH] ob-core: add org-confirm-babel-evaluate-cell custom variable
Date: Wed, 14 Dec 2022 10:24:30 -0800	[thread overview]
Message-ID: <CA+G3_PPo9+vy5Ls+N_xnXwXBGUsCDBkZZRx1grT=274Q9XffKg@mail.gmail.com> (raw)
In-Reply-To: <tncual$pde$1@ciao.gmane.io>

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

Interestingly, as I was looking through my notes in
orgstrap, I see my past self had found a macro
org-babel-one-header-arg-safe-p which pointed to
defconst org-babel-safe-header-args, but that is
a const and not really user configurable. Of course
the user could cl-setf on it, but it also only checks
strings and has no ability to e.g. see if the value of
(symbol-function #'identity) has changed since the
check function was defined. Two examples.

(let ((old-identity (symbol-function #'identity)))
  (cl-letf* (((symbol-function #'identity)
              (lambda (x) (message "there") x)))
    (identity 'hello)
    (equal (symbol-function #'identity) old-identity)))

(let ((old-and (symbol-function #'and)))
  (cl-letf* (((symbol-function #'old-andf) old-and)
             ((symbol-function #'and)
              (lambda (&rest args)
                (message "oops %s" args)
                (old-andf args))))
    (list
     (and)
     (and 1 2 3)
     (equal (symbol-function #'and) old-and))))

> Tom, does not the following allow to achieve the same without your patch?

It works if I have a closed set of things I want to allow but not if I
want to set it to nil to e.g. restore the old behavior (worse for
security but not as bad as setting ocbe to nil), e.g. if I'm
under duress and need to get something that used to work to
work again without the risk of automatically running dangerous
code, (e.g. blocks that might rm something).

> I know, it does not work, but I think it is due to (format "%S" cell)
> instead of passing cell directly in
>
> -                            '((:eval . yes)) nil (format "%S" cell)
>
> My point is that if some expression is safe for a variable value then it
> is safe for the source block body.

There is another use case here, which I alluded to in the previous
comment, which is that sometimes ocbe is the last line of defense
against running dangerous code. Ideally users would have set
:eval never on blocks like that to be sure, but if they don't you
don't want someone already trying to get something to work
to get too much to work.

Again, this is focused on the ocbec -> nil case.

> Have you ever seen the prompt for a table?

Err ... maybe? So the answer is probably no.

> I suppose, tables are the most prominent security issue related to
> unsolicited code execution:

For me it would be arbitrary expressions in the headers
of source blocks. Hard to know which one is more prevalent
across the population of org users.

> Max Nikulin to emacs-orgmode. Re: [BUG][Security] begin_src :var
> evaluated before the prompt to confirm execution. Fri, 28 Oct 2022
> 11:11:18 +0700. https://list.orgmode.org/tjfkp7$ggm$1@ciao.gmane.io
>
> I am still in doubts if
>
> 10e857d42 2022-10-28 11:09:50 +0800 Ihor Radchenko: org-babel-read: Obey
> `org-confirm-babel-evaluate'
>
> was an unambiguous improvement. Perhaps it just forces more users to set
> `org-confirm-babel-evaluate' to nil compromising their security to more
> severe degree.

Heh. It is always a hard balance to strike. In the context of that
thread having a variable that would find-file-literally for untrusted
org files by default might be useful.

Again, it is a pain. I can tell you from experience having written
the system that does something similar for orgstrap. There is
no safe way other than a user-maintained whitelist based on
file hashes, everything else can be spoofed in one way or another.

I suspect that once we have the machinery in this patch in
place we can look for some sane defaults. Note that the example
function we keep passing around isn't quite good enough because
someone could probably figure out how to rewrite the identity
function so we would need to make sure that it had not changed
since emacs was loaded (unlikely, but if I can image it someone
could surely do it).

[-- Attachment #2: Type: text/html, Size: 4437 bytes --]

  reply	other threads:[~2022-12-14 18:25 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 [this message]
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                         ` [SECURITY] Arbitrary code evaluation security in Org (was: [PATCH] ob-core: add org-confirm-babel-evaluate-cell custom variable) Ihor Radchenko
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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  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='CA+G3_PPo9+vy5Ls+N_xnXwXBGUsCDBkZZRx1grT=274Q9XffKg@mail.gmail.com' \
    --to=tgbugs@gmail.com \
    --cc=emacs-orgmode@gnu.org \
    --cc=manikulin@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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

	https://git.savannah.gnu.org/cgit/emacs/org-mode.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).