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

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

[lux]

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

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.

[-- Attachment #2: 0001-Fix-etags-local-command-injection-vulnerability.patch --]
[-- Type: text/x-patch, Size: 6233 bytes --]

From 30cec9dcbd67998ce2360d191044e8d97992f794 Mon Sep 17 00:00:00 2001
From: lu4nx <lx@shellcodes.org>
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


[-- Attachment #3: 0001-Fix-ruby-mode.el-local-command-injection-vulnerabili.patch --]
[-- Type: text/x-patch, Size: 992 bytes --]

From e1579b5ebde7c6935c84a1998e618d7c27c79731 Mon Sep 17 00:00:00 2001
From: Xi Lu <lx@shellcodes.org>
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

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

[lux]

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

Sorry, 0001-Fix-etags-local-command-injection-vulnerability.patch is a file I uploaded by mistake, please ignore

[-- Attachment #2: Type: text/html, Size: 112 bytes --]

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

[Dmitry Gutov]

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.