bug#60295: [PATCH] Fix htmlfontify.el command injection vulnerability

bug#60295: [PATCH] Fix htmlfontify.el command injection vulnerability

[lux]

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

Test information:
Emacs version: GNU Emacs 29.0.60
OS: Fedora Linux 37

htmlfontify.el has a command injection vulnerability:

(defcustom hfy-istext-command "file %s | sed -e 's@^[^:]*:[ \t]*@@'"
  :tag   "istext-command"
  :type  '(string))

(defun hfy-text-p (srcdir file)
  (let* ((cmd (format hfy-istext-command (expand-file-name file
srcdir))) (rsp (shell-command-to-string    cmd)))
    ...))

Parameter 'file' and parameter 'srcdir' come from external input, and 
parameters are not escape. So, if file name or directory name contains
shell characters and will be executed.

For example:

$ mkdir vul_test
$ cd vul_test
$ echo hello > ";uname>hack.txt#"
$ ls
;uname>hack.txt#

In Emacs, type M-x htmlfontify-copy-and-link-dir, and inputing vul_test
path, at this time, hack.txt is added to the vul_test directory:

$ ls
;uname>hack.txt#  hack.txt#
$ cat hack.txt\#
Linux

The attachment is the patch file, thanks.




[-- Attachment #2: 0001-Fix-htmlfontify.el-command-injection-vulnerability.patch --]
[-- Type: text/x-patch, Size: 924 bytes --]

From b97db7fc0d38595507ca78018724c769e873a469 Mon Sep 17 00:00:00 2001
From: Xi Lu <lx@shellcodes.org>
Date: Sat, 24 Dec 2022 16:28:54 +0800
Subject: [PATCH] Fix htmlfontify.el command injection vulnerability.

* lisp/htmlfontify.el
(hfy-text-p): Fix command injection vulnerability.
---
 lisp/htmlfontify.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/htmlfontify.el b/lisp/htmlfontify.el
index df4c6ab079..389b92939c 100644
--- a/lisp/htmlfontify.el
+++ b/lisp/htmlfontify.el
@@ -1850,7 +1850,7 @@ hfy-make-directory
 
 (defun hfy-text-p (srcdir file)
   "Is SRCDIR/FILE text?  Use `hfy-istext-command' to determine this."
-  (let* ((cmd (format hfy-istext-command (expand-file-name file srcdir)))
+  (let* ((cmd (format hfy-istext-command (shell-quote-argument (expand-file-name file srcdir))))
          (rsp (shell-command-to-string    cmd)))
     (string-match "text" rsp)))
 
-- 
2.38.1

bug#60295: [PATCH] Fix htmlfontify.el command injection vulnerability

[Stefan Kangas]

tags 60295 + security
thanks

lux <lx@shellcodes.org> writes:

> From b97db7fc0d38595507ca78018724c769e873a469 Mon Sep 17 00:00:00 2001
> From: Xi Lu <lx@shellcodes.org>
> Date: Sat, 24 Dec 2022 16:28:54 +0800
> Subject: [PATCH] Fix htmlfontify.el command injection vulnerability.
>
> * lisp/htmlfontify.el
> (hfy-text-p): Fix command injection vulnerability.
> ---
>  lisp/htmlfontify.el | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lisp/htmlfontify.el b/lisp/htmlfontify.el
> index df4c6ab079..389b92939c 100644
> --- a/lisp/htmlfontify.el
> +++ b/lisp/htmlfontify.el
> @@ -1850,7 +1850,7 @@ hfy-make-directory
>
>  (defun hfy-text-p (srcdir file)
>    "Is SRCDIR/FILE text?  Use `hfy-istext-command' to determine this."
> -  (let* ((cmd (format hfy-istext-command (expand-file-name file srcdir)))
> +  (let* ((cmd (format hfy-istext-command (shell-quote-argument (expand-file-name file srcdir))))
>           (rsp (shell-command-to-string    cmd)))
>      (string-match "text" rsp)))

Eli, is it okay to install this patch on the Emacs 29 branch?  It looks
safe, as it only adds shell quoting to a filename before it is fed to
`shell-command-to-string'.

But on master maybe we could avoid calling the shell altogether by using
something like this:

    (defun file-binary-p (filename)
      "Return t if FILENAME names a binary file.
    Return nil if FILENAME does not name a binary file, or if there
    was trouble determining whether FILENAME is a binary file."
      (when (and (file-readable-p filename)
                 (not (file-directory-p filename)))
        (catch 'binaryp
          (with-current-buffer (find-file-noselect filename t)
            (unwind-protect
                (throw 'binaryp (eq buffer-file-coding-system 'binary))
              (kill-buffer))))))

bug#60295: [PATCH] Fix htmlfontify.el command injection vulnerability

[Eli Zaretskii]

> Date: Sat, 24 Dec 2022 17:03:09 +0800
> From: lux <lx@shellcodes.org>
> 
> Test information:
> Emacs version: GNU Emacs 29.0.60
> OS: Fedora Linux 37
> 
> htmlfontify.el has a command injection vulnerability:
> 
> (defcustom hfy-istext-command "file %s | sed -e 's@^[^:]*:[ \t]*@@'"
>   :tag   "istext-command"
>   :type  '(string))
> 
> (defun hfy-text-p (srcdir file)
>   (let* ((cmd (format hfy-istext-command (expand-file-name file
> srcdir))) (rsp (shell-command-to-string    cmd)))
>     ...))
> 
> Parameter 'file' and parameter 'srcdir' come from external input, and 
> parameters are not escape. So, if file name or directory name contains
> shell characters and will be executed.
> 
> For example:
> 
> $ mkdir vul_test
> $ cd vul_test
> $ echo hello > ";uname>hack.txt#"
> $ ls
> ;uname>hack.txt#
> 
> In Emacs, type M-x htmlfontify-copy-and-link-dir, and inputing vul_test
> path, at this time, hack.txt is added to the vul_test directory:
> 
> $ ls
> ;uname>hack.txt#  hack.txt#
> $ cat hack.txt\#
> Linux
> 
> The attachment is the patch file, thanks.

Thanks, installed on the emacs-29 branch, and closing the bug.