GNU bug report logs - #60295
[PATCH] Fix htmlfontify.el command injection vulnerability

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 60295 in the body.
You can then email your comments to 60295 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: lux <lx <at> shellcodes.org>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Fix htmlfontify.el command injection vulnerability
Date: Sat, 24 Dec 2022 17:03:09 +0800

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

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.

[0001-Fix-htmlfontify.el-command-injection-vulnerability.patch (text/x-patch, attachment)]

Message #8 received at 60295 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: lux <lx <at> shellcodes.org>, 60295 <at> debbugs.gnu.org, eliz <at> gnu.org
Subject: Re: bug#60295: [PATCH] Fix htmlfontify.el command injection
 vulnerability
Date: Mon, 26 Dec 2022 19:03:35 +0000

tags 60295 + security
thanks

lux <lx <at> shellcodes.org> writes:

> From b97db7fc0d38595507ca78018724c769e873a469 Mon Sep 17 00:00:00 2001
> From: Xi Lu <lx <at> 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))))))

Message #15 received at 60295-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: lux <lx <at> shellcodes.org>
Cc: 60295-done <at> debbugs.gnu.org
Subject: Re: bug#60295: [PATCH] Fix htmlfontify.el command injection
 vulnerability
Date: Tue, 27 Dec 2022 16:11:21 +0200

> Date: Sat, 24 Dec 2022 17:03:09 +0800
> From: lux <lx <at> 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.

Previous Next

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