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

Previous Next

Full log

View this message in rfc822 format

From: lux <lx <at> shellcodes.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 61709 <at> debbugs.gnu.org
Subject: bug#61709: [PATCH] Security hardening: safely invoke `shell-command*' function.
Date: Thu, 23 Feb 2023 21:17:12 +0800

[Message part 1 (text/plain, inline)]

On Wed, 2023-02-22 at 17:29 +0200, Eli Zaretskii wrote:
> > Cc: Xi Lu <lx <at> shellcodes.org>
> > From: Xi Lu <lx <at> 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 (text/x-patch, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.