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?
