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

From: Eli Zaretskii <[email-masked]>
Date: Tue, 06 Dec 2022 14:55:09 +0200

> Date: Tue, 6 Dec 2022 15:48:10 +0800
> From: lux <[email-masked]>
> Cc: Stefan Kangas <[email-masked]>, [email-masked]
> 
> @@ -1716,8 +1717,12 @@ process_file_name (char *file, language *lang)
>  	  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);
> +          char *new_real_name = escape_shell_arg_string (real_name);
> +          char *new_tmp_name = escape_shell_arg_string (tmp_name);
> +          char *cmd1 = concat (compr->command, " ", new_real_name);
> +          char *cmd = concat (cmd1, " > ", new_tmp_name);
> +          free (new_real_name);
> +          free (new_tmp_name);
>  #endif

The "MSDOS || DOS_NT" case also needs a small change:

>  	  char *cmd = concat (cmd1, "\" > ", tmp_name);

This doesn't quote tmp_name; it should.

> +static char*
             ^^
There should be a space before "*".

> +      if (*p == '\'')
> +        {
> +          new_str[i+1] = '\\';
> +          new_str[i+2] = '\'';
> +          new_str[i+3] = '\'';
> +          i += 3;

I don't understand why you are adding ''\'' and not just \'.  Wouldn't the
latter work for some reason?

Thanks.