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

[lux <lx@shellcodes.org>]

[-- Attachment #1: Type: text/plain, Size: 1782 bytes --]

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.
> 
> 
> 

This patch went unaddressed for a long time, so just to be on the safe side, I
only remove the `filesets-select-command' function.


[-- Attachment #2: 0001-Removed-the-filesets-select-command-which-was-unused.patch --]
[-- Type: text/x-patch, Size: 2110 bytes --]

From 8f8db0851e9fd265a6bb106f3adf0168195162b8 Mon Sep 17 00:00:00 2001
From: Xi Lu <lx@shellcodes.org>
Date: Mon, 5 Feb 2024 13:41:13 +0800
Subject: [PATCH] Removed the `filesets-select-command', which was unused and
 unsafe.

* lisp/filesets.el: Removed the `filesets-select-command'.
---
 lisp/filesets.el | 22 +---------------------
 1 file changed, 1 insertion(+), 21 deletions(-)

diff --git a/lisp/filesets.el b/lisp/filesets.el
index 4e2de8fed1b..23a8dbc4e85 100644
--- a/lisp/filesets.el
+++ b/lisp/filesets.el
@@ -161,15 +161,6 @@ 'filesets-some
 (define-obsolete-function-alias 'filesets-member #'cl-member "28.1")
 (define-obsolete-function-alias 'filesets-sublist #'seq-subseq "28.1")
 
-(defun filesets-select-command (cmd-list)
-  "Select one command from CMD-LIST -- a string with space separated names."
-  (let ((this (shell-command-to-string
-	       (format "which --skip-alias %s 2> %s | head -n 1"
-		       cmd-list null-device))))
-    (if (equal this "")
-	nil
-      (file-name-nondirectory (substring this 0 (- (length this) 1))))))
-
 (defun filesets-which-command (cmd)
   "Call \"which CMD\"."
   (shell-command-to-string (format "which %s" cmd)))
@@ -546,18 +537,7 @@ filesets-commands
                                        (function :tag "Function"))))))
 
 (defcustom filesets-external-viewers
-  (let
-      ;; ((ps-cmd  (or (and (boundp 'my-ps-viewer) my-ps-viewer)
-      ;;    	    (filesets-select-command "ggv gv")))
-      ;;  (pdf-cmd (or (and (boundp 'my-ps-viewer) my-pdf-viewer)
-      ;;    	    (filesets-select-command "xpdf acroread")))
-      ;;  (dvi-cmd (or (and (boundp 'my-ps-viewer) my-dvi-viewer)
-      ;;    	    (filesets-select-command "xdvi tkdvi")))
-      ;;  (doc-cmd (or (and (boundp 'my-ps-viewer) my-doc-viewer)
-      ;;    	    (filesets-select-command "antiword")))
-      ;;  (pic-cmd (or (and (boundp 'my-ps-viewer) my-pic-viewer)
-      ;;    	    (filesets-select-command "gqview ee display"))))
-      ((ps-cmd  "ggv")
+  (let ((ps-cmd  "ggv")
        (pdf-cmd "xpdf")
        (dvi-cmd "xdvi")
        (doc-cmd "antiword")
-- 
2.43.0