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

From: lux <[email-masked]>
Date: Tue, 6 Dec 2022 23:49:05 +0800

On Tue, 06 Dec 2022 16:52:40 +0200
Eli Zaretskii <[email-masked]> wrote:

> Windows file names cannot include quote characters, so don't use
> them.  And it's TEMP value that you need to tweak, not the file names
> etags scans.

Thank you, fixed.
 
> I don't understand why you need an extra pair of quotes in the
> expanded string.
> 
>   $ echo \''hello; world'
>   'hello; world
> 
> As you see, the semi-colon was successfully hidden from the shell.
> 
> What am I missing?

$ echo Emacs > "'hello'world"
$ cat '\''hello\''world'     <---- use \'', error
cat: '\hello\world': No such file or directory
$ cat ''\''hello'\''world'    <---- use '\''
Emacs

You can also refer to:

1.
https://stackoverflow.com/questions/48970174/escape-single-quote-in-command-argument-to-sh-c

2. And I found a similar function in PHP:

$ cat test.php
<?php
	echo escapeshellarg("'hello'world");

$ php test.php
''\''hello'\''world'
>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.
---
 lib-src/etags.c | 63 +++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 58 insertions(+), 5 deletions(-)

diff --git a/lib-src/etags.c b/lib-src/etags.c
index d1d20858cd..ba0092cc63 100644
--- a/lib-src/etags.c
+++ b/lib-src/etags.c
@@ -401,6 +401,7 @@ #define xrnew(op, n, m) ((op) = xnrealloc (op, n, (m) * sizeof *(op)))
 static void put_entries (node *);
 static void cleanup_tags_file (char const * const, char const * const);
 
+static char *escape_shell_arg_string (char *);
 static void do_move_file (const char *, const char *);
 static char *concat (const char *, const char *, const char *);
 static char *skip_spaces (char *);
@@ -1713,13 +1714,16 @@ process_file_name (char *file, language *lang)
       else
 	{
 #if MSDOS || defined (DOS_NT)
-	  char *cmd1 = concat (compr->command, " \"", real_name);
-	  char *cmd = concat (cmd1, "\" > ", tmp_name);
+          int buf_len = strlen (compr->command) + strlen (" \"\" > \"\"") + strlen (real_name) + strlen (tmp_name) + 1;
+          char *cmd = xmalloc (buf_len);
+          snprintf (cmd, buf_len, "%s \"%s\" > \"%s\"", compr->command, real_name, tmp_name);
 #else
-	  char *cmd1 = concat (compr->command, " '", real_name);
-	  char *cmd = concat (cmd1, "' > ", tmp_name);
+          char *new_real_name = escape_shell_arg_string (real_name);
+          char *new_tmp_name = escape_shell_arg_string (tmp_name);
+          int buf_len = strlen (compr->command) + strlen ("  > ") + strlen (new_real_name) + strlen (new_tmp_name) + 1;
+          char *cmd = xmalloc (buf_len);
+          snprintf (cmd, buf_len, "%s %s > %s", compr->command, new_real_name, new_tmp_name);
 #endif
-	  free (cmd1);
 	  inf = (system (cmd) == -1
 		 ? NULL
 		 : fopen (tmp_name, "r" FOPEN_BINARY));
@@ -7707,6 +7711,55 @@ etags_mktmp (void)
   return templt;
 }
 
+/*
+ * Adds single quotes around a string, if found single quotes, escaped it.
+ * Return a newly-allocated string.
+ *
+ * For example:
+ * escape_shell_arg_string("test.txt") => 'test.txt'
+ * escape_shell_arg_string("'test.txt") => ''\''test.txt'
+ */
+static char *
+escape_shell_arg_string (char *str)
+{
+  char *p = str;
+  int need_space = 2;           /* ' at begin and end */
+
+  while (*p != '\0')
+    {
+      if (*p == '\'')
+        need_space += 4;        /* ' to '\'', length is 4 */
+      else
+        need_space++;
+
+      p++;
+    }
+
+  char *new_str = xnew (need_space + 1, char);
+  new_str[0] = '\'';
+  new_str[need_space-1] = '\'';
+
+  int i = 1;                    /* skip first byte */
+  p = str;
+  while (*p != '\0')
+    {
+      new_str[i] = *p;
+      if (*p == '\'')
+        {
+          new_str[i+1] = '\\';
+          new_str[i+2] = '\'';
+          new_str[i+3] = '\'';
+          i += 3;
+        }
+
+      i++;
+      p++;
+    }
+
+  new_str[need_space] = '\0';
+  return new_str;
+}
+
 static void
 do_move_file(const char *src_file, const char *dst_file)
 {
-- 
2.38.1