Re: [BUG][SECURITY] ob-sqlite header args allows execution of arbitrary shell commands

[Max Nikulin]

On 21/08/2023 14:04, Ihor Radchenko wrote:
> +(defconst org-shell-arg-literal (gensym "literal")

(opinion) Perhaps a better name exists. Maybe 
org-shell-arg-tag-unescaped (or unquoted)

> +  "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.
> +
> +ARGS can be nil, strings, (LITERAL STRING), or a list of

(opinion) I would give an example

    `(,ob-shell-arg-literal STRING)

to avoid "LITERAL" that is confusing from my point of view. Perhaps it 
is better to describe its purpose more clearly: prevent raw shell 
constructs in ob header arguments in Org documents unless they appear in 
evaluated expressions.

> +such elements.  LITERAL must be the value of `org-shell-arg-literal'.
> +
> +Strings will be quoted with `shell-quote-argument' while \(literal
> +STRING) will be used without quoting.  nil values will be ignored."
> +  (concat
> +   command (when command " ")
> +   (mapconcat
> +    #'identity
> +    (delq
> +     nil
> +     (mapcar
> +      (lambda (str-def)
> +        (pcase str-def
> +          (`(or nil "") nil)

An empty string may be an important argument. E.g.

    read -r -d "" var

allows to read values separated by null character (\0), e.g. from output 
of find -print0. That is why I would leave just nil.

> +         (when separator (format "-separator %s" separator))

Sorry, I made a typo previous time. It would not work. -separator and 
the separator character must *not* be combined into single argument. 
Sqlite does not support it.

    (and separator `("-separator" ,separator))

or (format "%s" separator) if it may have a type other than string.

I hope, this approach does not have unnoticed flaws.