Subject: bug#59817: closed (Re: bug#59817: [PATCH] Fix etags local command injection vulnerability)

From: [email-masked] (GNU bug Tracking System)
Date: Tue, 06 Dec 2022 16:16:02 +0000

Your bug report

#59817: [PATCH] Fix etags 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].

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

Subject: Re: bug#59817: [PATCH] Fix etags local command injection vulnerability
From: Eli Zaretskii <[email-masked]>
To: lux <[email-masked]>
Cc: [email-masked], [email-masked]
Date: Tue, 06 Dec 2022 18:14:43 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

> Date: Tue, 6 Dec 2022 23:49:05 +0800
> From: lux <[email-masked]>
> Cc: [email-masked], [email-masked]
> 
> >From d1dd12396b7d99ff93e6a846c96ae600addac847 Mon Sep 17 00:00:00 2001
> From: lu4nx <[email-masked]>
> Date: Tue, 6 Dec 2022 15:42:40 +0800
> Subject: [PATCH] Fix etags local command injection vulnerability
> 
> * lib-src/etags.c:
> 
> (escape_shell_arg_string): New function.

Thanks, installed with some minor changes.

Subject: [PATCH] Fix etags local command injection vulnerability
From: lux <[email-masked]>
To: [email-masked]
Date: Sun, 4 Dec 2022 21:51:13 +0800
List-Id: <debbugs-submit.debbugs.gnu.org>

Hi, this patch fix a new local command injection vulnerability in the
etags.c.

This vulnerability occurs in the following code:

	#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);
	       }

Vulnerability #1:

for tmp_name variable, the value from the etags_mktmp() function, this
function takes the value from the environment variable `TMPDIR`, `TEMP`
or `TMP`, but without checking the value. So, if then hacker can
control these environment variables, can execute the shell code.

Attack example:

$ ls
etags.c
$ zip etags.z etags.c
  adding: etags.c (deflated 72%)
$ tmpdir="/tmp/;uname -a;/"
$ mkdir $tmpdir
$ TMPDIR=$tmpdir etags *
sh: line 1: /tmp/: Is a directory
Linux mypc 6.0.10-300.fc37.x86_64 #1 SMP PREEMPT_DYNAMIC Sat Nov 26
16:55:13 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux sh: line 1: /etECggCJ:
No such file or directory etags: skipping inclusion of TAGS in self.

Vulnerability #2:

If the target file is a compressed file, execute system commands (such
as gzip, etc.), but do not check the file name. 

Attack example:

$ ls
etags.c
$ zip "';uname -a;'test.z" etags.c  <--- inject the shell code to
filename
adding: etags.c (deflated 72%)
$ etags *
gzip: .gz: No such file or directory
Linux mypc 6.0.10-300.fc37.x86_64 #1 SMP PREEMPT_DYNAMIC Sat Nov 26
16:55:13 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux sh: line 1: test.z:
command not found

I fix this vulnerability. By create a process, instead of call the
sh or cmd.exe, and this patch work the Linux, BSD and Windows.
>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