Subject: bug#60268: closed (Re: bug#60268: [PATCH] Fix ruby-mode.el local command injection vulnerability)

From: [email-masked] (GNU bug Tracking System)
Date: Fri, 23 Dec 2022 23:45:02 +0000

Your bug report

#60268: [PATCH] Fix ruby-mode.el local command injection vulnerability

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
60268: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=60268
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#60268: [PATCH] Fix ruby-mode.el local command injection vulnerability
From: Dmitry Gutov <[email-masked]>
To: lux <[email-masked]>, [email-masked]
Date: Sat, 24 Dec 2022 01:43:56 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

Version: 29.1

On 23/12/2022 06:56, lux wrote:
> In ruby-mode.el, the 'ruby-find-library-file' function have a local
> command injection vulnerability:
> 
> 	(defun ruby-find-library-file (&optional feature-name)
> 	  (interactive)
> 	  ...
> 	  (shell-command-to-string (concat "gem which "
> 	(shell-quote-argument feature-name))) ...)
> 
> The 'ruby-find-library-file' is a interactive function, and bound to the
> shortcut key C-c C-f. Inside the function, the external command 'gem' is
> called through 'shell-command-to-string', but the 'feature-name'
> parameters are not escape.
> 
> So, if the Ruby source file contains the following:
> 
> 	require 'irb;id'
> 
> and typing C-c C-f, there is a risk of executing unexpected orders, for
> example:
> 
> 	(ruby-find-library-file "irb;uname")
> 	#<buffer irb.rb
> 	Linux>
> 
> Although the probability of being exploited is low, but I think it's
> still necessary to avoid this kind of security problem.
> 
> The attachment is the patch file, thanks.

Thanks! Installed.

Subject: [PATCH] Fix ruby-mode.el local command injection vulnerability
From: lux <[email-masked]>
To: [email-masked]
Date: Fri, 23 Dec 2022 12:56:30 +0800
List-Id: <debbugs-submit.debbugs.gnu.org>

In ruby-mode.el, the 'ruby-find-library-file' function have a local
command injection vulnerability:

	(defun ruby-find-library-file (&optional feature-name)
	  (interactive)
	  ...
	  (shell-command-to-string (concat "gem which "
	(shell-quote-argument feature-name))) ...)

The 'ruby-find-library-file' is a interactive function, and bound to the
shortcut key C-c C-f. Inside the function, the external command 'gem' is
called through 'shell-command-to-string', but the 'feature-name'
parameters are not escape.

So, if the Ruby source file contains the following:

	require 'irb;id'

and typing C-c C-f, there is a risk of executing unexpected orders, for
example:

	(ruby-find-library-file "irb;uname")
	#<buffer irb.rb
	Linux>

Although the probability of being exploited is low, but I think it's
still necessary to avoid this kind of security problem.

The attachment is the patch file, thanks.
>From 30cec9dcbd67998ce2360d191044e8d97992f794 Mon Sep 17 00:00:00 2001
From: lu4nx <[email-masked]>
Date: Sun, 4 Dec 2022 21:18:29 +0800
Subject: [PATCH] Fix etags local command injection vulnerability

* lib-src/etags.c:

(decompress_file): New function
---
 lib-src/etags.c | 155 +++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 121 insertions(+), 34 deletions(-)

diff --git a/lib-src/etags.c b/lib-src/etags.c
index d1d20858cd..7c5faf8d6b 100644
--- a/lib-src/etags.c
+++ b/lib-src/etags.c
@@ -97,6 +97,7 @@ Copyright (C) 1984, 1987-1989, 1993-1995, 1998-2022 Free Software
 
 #ifdef WINDOWSNT
 # include <direct.h>
+# include <windows.h>
 # undef HAVE_NTGUI
 # undef  DOS_NT
 # define DOS_NT
@@ -124,6 +125,7 @@ Copyright (C) 1984, 1987-1989, 1993-1995, 1998-2022 Free Software
 #include <assert.h>
 #include <getopt.h>
 #include <regex.h>
+#include <sys/wait.h>
 
 /* Define CTAGS to make the program "ctags" compatible with the usual one.
  Leave it undefined to make the program "etags", which makes emacs-style
@@ -255,7 +257,7 @@ #define xrnew(op, n, m) ((op) = xnrealloc (op, n, (m) * sizeof *(op)))
 typedef struct
 {
   const char *suffix;           /* file name suffix for this compressor */
-  const char *command;		/* takes one arg and decompresses to stdout */
+  const char *command;		/* uncompress command */
 } compressor;
 
 typedef struct
@@ -391,6 +393,7 @@ #define xrnew(op, n, m) ((op) = xnrealloc (op, n, (m) * sizeof *(op)))
 static _Noreturn void pfatal (const char *);
 static void add_node (node *, node **);
 
+static bool decompress_file (const char *, const char *, const char *);
 static void process_file_name (char *, language *);
 static void process_file (FILE *, char *, language *);
 static void find_entries (FILE *);
@@ -527,16 +530,16 @@ #define STDIN 0x1001		/* returned by getopt_long on --parse-stdin */
 };
 
 static compressor compressors[] =
-{
-  { "z", "gzip -d -c"},
-  { "Z", "gzip -d -c"},
-  { "gz", "gzip -d -c"},
-  { "GZ", "gzip -d -c"},
-  { "bz2", "bzip2 -d -c" },
-  { "xz", "xz -d -c" },
-  { "zst", "zstd -d -c" },
-  { NULL }
-};
+  {
+    { "z", "gzip" },
+    { "Z", "gzip" },
+    { "gz", "gzip" },
+    { "GZ", "gzip" },
+    { "bz2", "bzip2" },
+    { "xz", "xz" },
+    { "zst", "zstd" },
+    { NULL }
+  };
 
 /*
  * Language stuff.
@@ -1621,7 +1624,6 @@ process_file_name (char *file, language *lang)
   compressor *compr;
   char *compressed_name, *uncompressed_name;
   char *ext, *real_name UNINIT, *tmp_name UNINIT;
-  int retval;
 
   canonicalize_filename (file);
   if (streq (file, tagfile) && !streq (tagfile, "-"))
@@ -1712,37 +1714,29 @@ process_file_name (char *file, language *lang)
 	inf = NULL;
       else
 	{
-#if MSDOS || defined (DOS_NT)
-	  char *cmd1 = concat (compr->command, " \"", real_name);
-	  char *cmd = concat (cmd1, "\" > ", tmp_name);
-#else
-	  char *cmd1 = concat (compr->command, " '", real_name);
-	  char *cmd = concat (cmd1, "' > ", tmp_name);
-#endif
-	  free (cmd1);
-	  inf = (system (cmd) == -1
-		 ? NULL
-		 : fopen (tmp_name, "r" FOPEN_BINARY));
-	  free (cmd);
-	}
-
-      if (!inf)
-	{
-	  perror (real_name);
-	  goto cleanup;
+          inf = (!decompress_file (compr->command, real_name, tmp_name)
+                 ? NULL
+                 : fopen (tmp_name, "r" FOPEN_BINARY));
+          if (!inf)
+            {
+              perror (real_name);
+              goto cleanup;
+            }
 	}
     }
 
   process_file (inf, uncompressed_name, lang);
 
-  retval = fclose (inf);
+  if (fclose (inf) < 0)
+    pfatal (file);
+
   if (real_name == compressed_name)
     {
-      remove (tmp_name);
+      if (remove (tmp_name) == -1)
+        pfatal (tmp_name);
+
       free (tmp_name);
     }
-  if (retval < 0)
-    pfatal (file);
 
  cleanup:
   if (compressed_name != file)
@@ -1754,6 +1748,99 @@ process_file_name (char *file, language *lang)
   return;
 }
 
+/*
+ * Specify a decompression command and write the decompression content to a new file.
+ * On success, true is returned.
+ */
+static bool
+decompress_file (const char *command, const char *input_file, const char *output_file)
+{
+#ifdef DOS_NT
+  SECURITY_ATTRIBUTES sa;
+  sa.nLength = sizeof (SECURITY_ATTRIBUTES);
+  sa.lpSecurityDescriptor = NULL;
+  sa.bInheritHandle = true;
+
+  HANDLE hFile = CreateFile (output_file, GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_DELETE, &sa, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
+
+  if (hFile == INVALID_HANDLE_VALUE)
+    {
+      perror (output_file);
+      return false;
+    }
+
+  PROCESS_INFORMATION pi;
+  STARTUPINFO si;
+  ZeroMemory (&si, sizeof (si));
+  ZeroMemory (&pi, sizeof (pi));
+  si.cb = sizeof (STARTUPINFO);
+  si.dwFlags = STARTF_USESTDHANDLES;
+  si.hStdInput = NULL;
+  si.hStdOutput = hFile;
+  si.wShowWindow = SW_HIDE;
+
+  char *cmd = concat (command, " -d -c ", input_file);
+  if (!CreateProcess (NULL, cmd, NULL, NULL, TRUE, CREATE_NO_WINDOW, NULL, NULL, &si, &pi))
+    {
+      perror ("CreateProcess error");
+      return false;
+    }
+
+  WaitForSingleObject(pi.hProcess, INFINITE);
+
+  CloseHandle (pi.hProcess);
+  CloseHandle (pi.hThread);
+  CloseHandle (hFile);
+  return true;
+#else
+  int out_f;
+  if ((out_f = open (output_file, O_CREAT | O_WRONLY)) == -1)
+    {
+      perror (output_file);
+      return false;
+    }
+
+  pid_t pid = fork ();
+  if (pid == -1)
+    {
+      perror ("fork");
+      return false;
+    }
+
+  if (pid == 0)
+    {
+      if (dup2 (out_f, STDOUT_FILENO) == -1)
+        {
+          perror ("dup2 stdout error");
+          exit (EXIT_FAILURE);
+        }
+
+      char *command_args[] = { (char *) command, (char *) "-d", (char *) "-c", (char *) input_file, NULL };
+      if (execvp (command, command_args) == -1)
+        {
+          perror ("cannot execute the decompress command");
+          exit (EXIT_FAILURE);
+        }
+
+      exit (EXIT_SUCCESS);
+    }
+
+  if (waitpid (pid, NULL, 0) == -1)
+    {
+      perror ("waitpid error");
+      return false;
+    }
+
+  if (close (out_f) == -1)
+    {
+      perror ("close error");
+      return false;
+    }
+
+  return true;
+#endif
+}
+
 static void
 process_file (FILE *fh, char *fn, language *lang)
 {
-- 
2.38.1

>From e1579b5ebde7c6935c84a1998e618d7c27c79731 Mon Sep 17 00:00:00 2001
From: Xi Lu <[email-masked]>
Date: Fri, 23 Dec 2022 12:52:48 +0800
Subject: [PATCH] Fix ruby-mode.el local command injection vulnerability.

* lisp/progmodes/ruby-mode.el
(ruby-find-library-file): Fix local command injection vulnerability.
---
 lisp/progmodes/ruby-mode.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
index 1f3e9b6ae7..a4aa61905e 100644
--- a/lisp/progmodes/ruby-mode.el
+++ b/lisp/progmodes/ruby-mode.el
@@ -1899,7 +1899,7 @@ ruby-find-library-file
       (setq feature-name (read-string "Feature name: " init))))
   (let ((out
          (substring
-          (shell-command-to-string (concat "gem which " feature-name))
+          (shell-command-to-string (concat "gem which " (shell-quote-argument feature-name)))
           0 -1)))
     (if (string-match-p "\\`ERROR" out)
         (user-error "%s" out)
-- 
2.38.1