bug#61709: [PATCH] Security hardening: safely invoke `shell-command*' function.

[lux]

On Wed, 2023-02-22 at 17:29 +0200, Eli Zaretskii wrote:
> > Cc: Xi Lu <lx@shellcodes.org>
> > From: Xi Lu <lx@shellcodes.org>
> > Date: Wed, 22 Feb 2023 22:35:54 +0800
> > 
> >  (defun filesets-which-command-p (cmd)
> >    "Call \"which CMD\" and return non-nil if the command was
> > found."
> > @@ -1264,9 +1265,11 @@ filesets-spawn-external-viewer
> >                   (funcall vwr file)
> >                   nil)
> >                  (co-flag
> > -                 (shell-command-to-string (format "%s %s" vwr
> > args)))
> > +                 (shell-command-to-string (shell-quote-argument
> > +                                            (format "%s %s" vwr
> > args))))
> >                  (t
> > -                 (shell-command (format "%s %s&" vwr args))
> > +                 (shell-command (shell-quote-argument
> > +                                  (format "%s %s&" vwr args)))
> >                   nil))))
> 
> These two cannot be right: you are quoting several separate
> command-line arguments.
> 
> >           (if co-flag
> >               (progn
> > @@ -1578,7 +1581,7 @@ filesets-run-cmd
> >                                    " "))
> >                                  (cmd (concat fn " " args)))
> >                             (filesets-cmd-show-result
> > -                            cmd (shell-command-to-string cmd))))
> > +                            cmd (shell-command-to-string (shell-
> > quote-argument cmd)))))
> >                          ((symbolp fn)
> >                           (apply fn
> >                                  (mapcan (lambda (this)
> 
> I think this is also wrong: cmd is not a single word.
> 
> In general, you cannot quote arbitrary parts of a shell command, you
> can only quote each command-line argument separately.
> 
> 
> 

You're right, thank you. I rewrited this patch.

Let me briefly explain this patch:

1. I think `filesets-select-command' not need fixed, because it not
used, and I cleaned up relevant old comments in `filesets-external-
viewers'.

2. Using `shell-quote-argument' to replace `filesets-quote' and
`(format "%S" ...)'. Because in the shell, double quotation marks can
still execute unexpected code, such as $(), `command` and $var.

0001-Security-hardening-safely-invoke-shell-command-funct.patch
Description: Text Data