From: Max Nikulin <manikulin@gmail.com>
To: emacs-orgmode@gnu.org
Subject: Re: [BUG][SECURITY] ob-sqlite header args allows execution of arbitrary shell commands
Date: Mon, 28 Aug 2023 15:15:15 +0700 [thread overview]
Message-ID: <uchl2l$r06$1@ciao.gmane.io> (raw)
In-Reply-To: <87y1i31kb3.fsf@localhost>
On 22/08/2023 16:46, Ihor Radchenko wrote:
> See the updated version of the patches attached.
Thank you, I do not see apparent issues with code any more. Commit
message needs an update, apostrophes in the doc string should be
escaped. Feel free to ignore other comments since there are other issues
and investing excessive time into polishing of this one is not reasonable.
> Subject: [PATCH 1/2] org-macs: New common API function to quote shell
> arguments
>
> * lisp/org-macs.el (org-shell-arg-literal): New auxiliary constant.
^^^^^^^^^^^^^^^^^^^^^
You have changed its name.
> (org-make-shell-command): New function that returns shell command
> built from individual shell arguments, escaping them to prevent
> malicious code execution.
...
> +++ b/lisp/org-macs.el
> @@ -1593,6 +1593,46 @@ (defun org-sxhash-safe (obj &optional counter)
> (puthash hash obj org-sxhash-objects)
> (puthash obj hash org-sxhash-hashes)))))
>
> +(defconst org-shell-arg-tag-unescaped (gensym "literal")
> + "Symbol to be used to mark shell arguments that should not be escaped.
> +See `org-make-shell-command'.")
> +(defun org-make-shell-command (command &rest args)
> + "Build safe shell command string to run COMMAND with ARGS.
> +
> +The resulting shell command is safe against malicious shell expansion.
> +
> +This function is used to avoid unexpected shell expansion when
> +building shell command using header arguments from Org babel blocks.
> +
> +ARGS can be nil, strings, `(,org-shell-arg-tag-unescaped STRING), or a
add \\= before ` and ', otherwise help formatter makes them "pretty".
> +list of such elements. For example,
> +
> + (let ((files '(\"a.txt\" \"b.txt\" nil \"$HOME.txt\")))
> + `(org-make-shell-command \"command\" \"-l\"
> + \"value with spaces\"
> + (,org-shell-arg-tag-unescaped \"$HOME\")
> + (mapcar #'identity files)))
Is `mapcar' necessary here? Anyway `delq' is called on another result of
`mapcar', so the function should not do any destructive list modification.
An idea that may be ignored: make the constant internal and add
(defsubst org-make-shell-command-unescaped (arg)
(list org--shell-arg-tag-unescaped arg))
to avoid `, noise in `(,org-shell-arg-tag-unescaped STRING).
> +will shell-escape \"-l\", \"value with spaces\", and each non-nil member of
There is nothing to escape in "-l".
Perhaps it deserves a mention that COMMAND is passed unquoted to be
suitable for commands with arguments as defcustom user option values. To
escape it pass nil as fist argument and add COMMAND before ARGS.
> +FILES list, but leave \"$HOME\" to be expanded."
...by shell.
> Subject: [PATCH 2/2] org-babel-execute:sqlite: Fix shell arg expansion
> vulnerability
>
> - (org-fill-template
Should an explicit warning be added to `org-fill-template' that enough
care is required to escape values if it is used to build a shell command?
next prev parent reply other threads:[~2023-08-28 8:16 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-11 10:59 [BUG][SECURITY] ob-sqlite header args allows execution of arbitrary shell commands Max Nikulin
2023-08-13 7:52 ` Ihor Radchenko
2023-08-17 16:11 ` Max Nikulin
2023-08-18 8:43 ` Ihor Radchenko
2023-08-18 10:56 ` Max Nikulin
2023-08-18 11:05 ` Ihor Radchenko
2023-08-19 5:58 ` Max Nikulin
2023-08-21 7:04 ` Ihor Radchenko
2023-08-21 15:05 ` Max Nikulin
2023-08-22 9:46 ` Ihor Radchenko
2023-08-28 8:15 ` Max Nikulin [this message]
2023-08-29 8:02 ` Ihor Radchenko
2023-08-21 7:09 ` [SECURITY] Shell expansion of babel header args (was: [BUG][SECURITY] ob-sqlite header args allows execution of arbitrary shell commands) Ihor Radchenko
2023-08-17 16:29 ` [BUG][SECURITY] ob-sqlite header args allows execution of arbitrary shell commands 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='uchl2l$r06$1@ciao.gmane.io' \
--to=manikulin@gmail.com \
--cc=emacs-orgmode@gnu.org \
/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).