From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <emacs-orgmode-bounces+larch=yhetil.org@gnu.org>
Received: from mp11.migadu.com ([2001:41d0:8:6d80::])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	by ms5.migadu.com with LMTPS
	id +Aw5ISEVmmO9JQAAbAwnHQ
	(envelope-from <emacs-orgmode-bounces+larch=yhetil.org@gnu.org>)
	for <larch@yhetil.org>; Wed, 14 Dec 2022 19:25:37 +0100
Received: from aspmx1.migadu.com ([2001:41d0:8:6d80::])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	by mp11.migadu.com with LMTPS
	id gCw3ISEVmmNymwAA9RJhRA
	(envelope-from <emacs-orgmode-bounces+larch=yhetil.org@gnu.org>)
	for <larch@yhetil.org>; Wed, 14 Dec 2022 19:25:37 +0100
Received: from lists.gnu.org (lists.gnu.org [209.51.188.17])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by aspmx1.migadu.com (Postfix) with ESMTPS id 4D7271B4DC
	for <larch@yhetil.org>; Wed, 14 Dec 2022 19:25:37 +0100 (CET)
Received: from localhost ([::1] helo=lists1p.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.90_1)
	(envelope-from <emacs-orgmode-bounces@gnu.org>)
	id 1p5WQw-0001pc-Rw; Wed, 14 Dec 2022 13:24:46 -0500
Received: from eggs.gnu.org ([2001:470:142:3::10])
 by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <tgbugs@gmail.com>) id 1p5WQv-0001p9-Li
 for emacs-orgmode@gnu.org; Wed, 14 Dec 2022 13:24:45 -0500
Received: from mail-yb1-xb36.google.com ([2607:f8b0:4864:20::b36])
 by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128)
 (Exim 4.90_1) (envelope-from <tgbugs@gmail.com>) id 1p5WQt-0004Hk-B3
 for emacs-orgmode@gnu.org; Wed, 14 Dec 2022 13:24:45 -0500
Received: by mail-yb1-xb36.google.com with SMTP id e141so769456ybh.3
 for <emacs-orgmode@gnu.org>; Wed, 14 Dec 2022 10:24:42 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;
 h=cc:to:subject:message-id:date:from:in-reply-to:references
 :mime-version:from:to:cc:subject:date:message-id:reply-to;
 bh=/R/XXJO3R+Qw/Mkq3M5s2zg1I5XbVlyol1v1KTtNa90=;
 b=CjCDXFXm5zf5qKBx9Askxl9JwLoBDGsWtS+DLof1ObJOgwuTacNbZpexlRpbiPrizb
 +Bqm4XaeqoL+/8hOy3bBFhb9sSs7YTIi1yuvN9IqTti7KYAIe1nueevLO1d8mC7c2iFd
 LUBn6krbcS6ze5NVt+xouA8sCVHOzmNWvmWiB1v+bFGbRo0u0Iy9t6ZjXNc8mhpPynkn
 do7kCmie7eesNDZp1Rfvy/rih1NBZrYd4nBBQgJMq1yC958lLDalN61oJJS9q4rCuIx7
 RoT/+2Uo2tmFNxXpP1Q3lf7jE3bEPn25tKbHFTLHFyZ4P6U92H6FdLdwceiQLpg75bZE
 ihtg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20210112;
 h=cc:to:subject:message-id:date:from:in-reply-to:references
 :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id
 :reply-to;
 bh=/R/XXJO3R+Qw/Mkq3M5s2zg1I5XbVlyol1v1KTtNa90=;
 b=EzBdJNxv9dDWfd8IKb+qEDik6RZMRvdLfYLamBHkn0jw7fOqTrCHs49au2dPJFzaBD
 IBmTKMrfyuVTRzrt13tBS+F6FYdfGS+vG4Ayy/nfljX2HUnubKY2Md4+N3nbmF6gzUbB
 01cfLtL1eYFI2UFTiA0qv2CXzPKlU37L5h4FS9OPO3L9qZxT+w9zR+YNOyN+YmjDDGMR
 f7tHvVaiNSocFsPIrN51UjIxQsZ8PPcmKjig7hGBYbeJmYEeLeGRBreOuPxZYGQlodQN
 fchijXdZsuc7mgFBNxKrPHMTiO+ZAIfXTORlGlr4M4I2CyMivgySheT5iJ0GaGhU3bxv
 qPZQ==
X-Gm-Message-State: AFqh2kp3HEMYneg24rfdNBw3+WYLrNR2MF7zcAEZ802+vCZJHtG84KDS
 /BCO1l9+mFEH50MftbKPuXDJHbybGlzanWcBBti1q2CYcSY=
X-Google-Smtp-Source: AA0mqf4Yjg27kbXmChtezDM6fbwfigyX1entZQ38jaeQ3oo1X6JLirOEli4bn05Lg7FK68SYKKdqBMa/azHwcLAxXo0=
X-Received: by 2002:a05:6902:4a9:b0:731:7af4:cc3e with SMTP id
 r9-20020a05690204a900b007317af4cc3emr547175ybs.368.1671042281759; Wed, 14 Dec
 2022 10:24:41 -0800 (PST)
MIME-Version: 1.0
References: <CA+G3_PNmnJ-ehnYOBkaOOsyNjeb-OJyoy+sg_g5v3AZVGiNoXg@mail.gmail.com>
 <tn3h08$1099$1@ciao.gmane.io>
 <CA+G3_PNHe3J+PHzv_L+X1DR66TGc3sW5FxiJC5HqDd57N75P0w@mail.gmail.com>
 <87359ld5ye.fsf@kyleam.com>
 <CA+G3_PPEbiBFvADivF++x_c6s8hKtyTcy3nmBtTMd_OhBDDPyw@mail.gmail.com>
 <874ju0j538.fsf@localhost>
 <CA+G3_PMwyRrjwJp_AGxnV8P7LbqiPkGfLiQY4rziUo-xcjAUaA@mail.gmail.com>
 <tna9dn$2c9$1@ciao.gmane.io>
 <CA+G3_PN+fiEAbPvCp9virsqOK-F0utSoypSpVtksySOrqTvVnw@mail.gmail.com>
 <tncual$pde$1@ciao.gmane.io>
In-Reply-To: <tncual$pde$1@ciao.gmane.io>
From: Tom Gillespie <tgbugs@gmail.com>
Date: Wed, 14 Dec 2022 10:24:30 -0800
Message-ID: <CA+G3_PPo9+vy5Ls+N_xnXwXBGUsCDBkZZRx1grT=274Q9XffKg@mail.gmail.com>
Subject: Re: [PATCH] ob-core: add org-confirm-babel-evaluate-cell custom
 variable
To: Max Nikulin <manikulin@gmail.com>
Cc: emacs-orgmode@gnu.org
Content-Type: multipart/alternative; boundary="0000000000009bedd405efcdd9e7"
Received-SPF: pass client-ip=2607:f8b0:4864:20::b36;
 envelope-from=tgbugs@gmail.com; helo=mail-yb1-xb36.google.com
X-Spam_score_int: -20
X-Spam_score: -2.1
X-Spam_bar: --
X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1,
 DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001,
 HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001,
 SPF_PASS=-0.001 autolearn=ham autolearn_force=no
X-Spam_action: no action
X-BeenThere: emacs-orgmode@gnu.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "General discussions about Org-mode." <emacs-orgmode.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/emacs-orgmode>,
 <mailto:emacs-orgmode-request@gnu.org?subject=unsubscribe>
List-Archive: <https://lists.gnu.org/archive/html/emacs-orgmode>
List-Post: <mailto:emacs-orgmode@gnu.org>
List-Help: <mailto:emacs-orgmode-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/emacs-orgmode>,
 <mailto:emacs-orgmode-request@gnu.org?subject=subscribe>
Errors-To: emacs-orgmode-bounces+larch=yhetil.org@gnu.org
Sender: emacs-orgmode-bounces+larch=yhetil.org@gnu.org
X-Migadu-Country: US
X-Migadu-Flow: FLOW_IN
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org;
	s=key1; t=1671042337;
	h=from:from:sender:sender:reply-to:subject:subject:date:date:
	 message-id:message-id:to:to:cc:cc:mime-version:mime-version:
	 content-type:content-type:in-reply-to:in-reply-to:
	 references:references:list-id:list-help:list-unsubscribe:
	 list-subscribe:list-post:dkim-signature;
	bh=/R/XXJO3R+Qw/Mkq3M5s2zg1I5XbVlyol1v1KTtNa90=;
	b=GBIBzUc02vAWfCw/HL1ges0v8ymmoIz8q6JtbuYOcOjQcbwZHIj479lOSQD8ayw2Kr2aIG
	xyFuH3Ltjp8WDexu4rg8i+NoQw276r85CJPOW8EEoUuA2KV9M6mddjksiZtE9AYEN0wq4D
	QwZeZufDOaEMq0JgXvJRhAQfyhF/DI7kMk4qRuQAVvq/ty/uaT4fJ1T9v9nbq8aOe0Bq3A
	aBaanzmhSPtFqgqV0kepHMP59kSnEPVb9wNJgn8q8/mxotyuvhRrMi+gFm2ohQ1fEpwXTc
	elNusHLCG3WILHrklKKQFGI1txWZUkyDKVDYP6tIH1fxelCudYXaCgMral+eiQ==
ARC-Authentication-Results: i=1;
	aspmx1.migadu.com;
	dkim=pass header.d=gmail.com header.s=20210112 header.b=CjCDXFXm;
	spf=pass (aspmx1.migadu.com: domain of "emacs-orgmode-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="emacs-orgmode-bounces+larch=yhetil.org@gnu.org";
	dmarc=pass (policy=none) header.from=gmail.com
ARC-Seal: i=1; s=key1; d=yhetil.org; t=1671042337; a=rsa-sha256; cv=none;
	b=VkU1mVfQ3rirrYsZhwN4HkO52kyOOawWNjUaHAewKqPD2OGUNQyacdUUgtujoaw3hLmTf5
	vm3zl3TkD4XnadRl8LP3rLc4WoqSNjQac5Y2UDC59PTunBPbMePwIfwqTEYtvIK7ahld92
	uC8hc3BiTRm9tK+1m8PqSRKLw6RTl+YfTMCpXkWp2sX2+8F5K77Cf+q2DFoheQN2sxz4az
	cIGef/1P+x8MV8zmHzFaLKURZxN9D0tGF+AS9e0pZsk8Pqsy2kSFXB8cHT4KePMb9PyHq/
	4wpRlWHatU2vX/O3Kju6jr2Jtg77jgOoW5zMjESeNCPjG1sGnPqOCgTMiZ5yjw==
X-Migadu-Spam-Score: -7.25
X-Spam-Score: -7.25
X-Migadu-Queue-Id: 4D7271B4DC
X-Migadu-Scanner: scn0.migadu.com
Authentication-Results: aspmx1.migadu.com;
	dkim=pass header.d=gmail.com header.s=20210112 header.b=CjCDXFXm;
	spf=pass (aspmx1.migadu.com: domain of "emacs-orgmode-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="emacs-orgmode-bounces+larch=yhetil.org@gnu.org";
	dmarc=pass (policy=none) header.from=gmail.com
X-TUID: 2g48ihrKPgz/

--0000000000009bedd405efcdd9e7
Content-Type: text/plain; charset="UTF-8"

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

--0000000000009bedd405efcdd9e7
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">Interestingly, as I was looking through my notes in<br>org=
strap, I see my past self had found a macro<br>org-babel-one-header-arg-saf=
e-p which pointed to<br>defconst org-babel-safe-header-args, but that is<br=
>a const and not really user configurable. Of course<br>the user could cl-s=
etf on it, but it also only checks<br>strings and has no ability to e.g. se=
e if the value of<br>(symbol-function #&#39;identity) has changed since the=
<br>check function was defined. Two examples.<br><br>(let ((old-identity (s=
ymbol-function #&#39;identity)))<br>=C2=A0 (cl-letf* (((symbol-function #&#=
39;identity)<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (lambda (x=
) (message &quot;there&quot;) x)))<br>=C2=A0 =C2=A0 (identity &#39;hello)<b=
r>=C2=A0 =C2=A0 (equal (symbol-function #&#39;identity) old-identity)))<br>=
<br>(let ((old-and (symbol-function #&#39;and)))<br>=C2=A0 (cl-letf* (((sym=
bol-function #&#39;old-andf) old-and)<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
 =C2=A0 =C2=A0((symbol-function #&#39;and)<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 (lambda (&amp;rest args)<br>=C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (message &quot;oops %s&quot; args)<br>=C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (old-andf args))))<br>=
=C2=A0 =C2=A0 (list<br>=C2=A0 =C2=A0 =C2=A0(and)<br>=C2=A0 =C2=A0 =C2=A0(an=
d 1 2 3)<br>=C2=A0 =C2=A0 =C2=A0(equal (symbol-function #&#39;and) old-and)=
)))<br><br>&gt; Tom, does not the following allow to achieve the same witho=
ut your patch?<br><br>It works if I have a closed set of things I want to a=
llow but not if I<br>want to set it to nil to e.g. restore the old behavior=
 (worse for<br>security but not as bad as setting ocbe to nil), e.g. if I&#=
39;m<br>under duress and need to get something that used to work to<br>work=
 again without the risk of automatically running dangerous<br>code, (e.g. b=
locks that might rm something).<br><br>&gt; I know, it does not work, but I=
 think it is due to (format &quot;%S&quot; cell)<br>&gt; instead of passing=
 cell directly in<br>&gt;<br>&gt; - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0&#39;((:eval . y=
es)) nil (format &quot;%S&quot; cell)<br>&gt;<br>&gt; My point is that if s=
ome expression is safe for a variable value then it<br>&gt; is safe for the=
 source block body.<br><br>There is another use case here, which I alluded =
to in the previous<br>comment, which is that sometimes ocbe is the last lin=
e of defense<br>against running dangerous code. Ideally users would have se=
t<br>:eval never on blocks like that to be sure, but if they don&#39;t you<=
br>don&#39;t want someone already trying to get something to work<br>to get=
 too much to work.<br><br>Again, this is focused on the ocbec -&gt; nil cas=
e.<br><br>&gt; Have you ever seen the prompt for a table?<br><br>Err ... ma=
ybe? So the answer is probably no.<br><br>&gt; I suppose, tables are the mo=
st prominent security issue related to<br>&gt; unsolicited code execution:<=
br><br>For me it would be arbitrary expressions in the headers<br>of source=
 blocks. Hard to know which one is more prevalent<br>across the population =
of org users.<br><br>&gt; Max Nikulin to emacs-orgmode. Re: [BUG][Security]=
 begin_src :var<br>&gt; evaluated before the prompt to confirm execution. F=
ri, 28 Oct 2022<br>&gt; 11:11:18 +0700. <a href=3D"https://list.orgmode.org=
/tjfkp7$ggm$1@ciao.gmane.io">https://list.orgmode.org/tjfkp7$ggm$1@ciao.gma=
ne.io</a><br>&gt;<br>&gt; I am still in doubts if<br>&gt;<br>&gt; 10e857d42=
 2022-10-28 11:09:50 +0800 Ihor Radchenko: org-babel-read: Obey<br>&gt; `or=
g-confirm-babel-evaluate&#39;<br>&gt;<br>&gt; was an unambiguous improvemen=
t. Perhaps it just forces more users to set<br>&gt; `org-confirm-babel-eval=
uate&#39; to nil compromising their security to more<br>&gt; severe degree.=
<br><br>Heh. It is always a hard balance to strike. In the context of that<=
br>thread having a variable that would find-file-literally for untrusted<br=
>org files by default might be useful.<br><br>Again, it is a pain. I can te=
ll you from experience having written<br>the system that does something sim=
ilar for orgstrap. There is<br>no safe way other than a user-maintained whi=
telist based on<br>file hashes, everything else can be spoofed in one way o=
r another.<br><br>I suspect that once we have the machinery in this patch i=
n<br>place we can look for some sane defaults. Note that the example<br>fun=
ction we keep passing around isn&#39;t quite good enough because<br>someone=
 could probably figure out how to rewrite the identity<br>function so we wo=
uld need to make sure that it had not changed<br>since emacs was loaded (un=
likely, but if I can image it someone<br>could surely do it).</div>

--0000000000009bedd405efcdd9e7--