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

From: Eli Zaretskii <[email-masked]>
Date: Sun, 04 Dec 2022 16:39:10 +0200

> Date: Sun, 4 Dec 2022 21:51:13 +0800
> From: lux <[email-masked]>
> 
> 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.

Thanks, but no, thanks.  This cure is worse than the disease.  Let's please
find simpler, more robust solutions.  It TMPDIR is a problem, let's use a
file whose name is hard-coded in the etags.c source, or quote the name when
we pass it to the shell.  If we suspect someone could disguise shell
commands as file names, let's quote the file names we pass to the shell with
'...' to prevent that.  Etc. etc. -- let's use simple solutions that don't
drastically change the code.

Please understand: etags is a stable program.  I'm not interested in changes
that modify its design or implementation in such drastic ways.