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

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

> Date: Tue, 6 Dec 2022 21:11:35 +0800
> From: lux <[email-masked]>
> Cc: [email-masked], [email-masked]
> 
> On Tue, 06 Dec 2022 14:55:09 +0200
> Eli Zaretskii <[email-masked]> wrote:
> 
> > The "MSDOS || DOS_NT" case also needs a small change:
> > 
> > >  	  char *cmd = concat (cmd1, "\" > ", tmp_name);  
> > 
> > This doesn't quote tmp_name; it should.
> 
> Because double quotes have been used here

The double quotes are only around real_name, but not around tmp_name.  One
of the issues you originally described was a bogus value of the TEMP
environment variable, which gets used in etags_mktmp that produces tmp_name.

> I have not reproduced this
> vulnerability in Windows, so I have not dealt:
> 
> $ touch "etags.c\" && ipconfig \".z"
> $ ./etags.exe "etags.c\" && ipconfig \".z"
> etags.c" && ipconfig ".z: Invalid argument

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.

> > I don't understand why you are adding ''\'' and not just \'.
> > Wouldn't the latter work for some reason?
> > 
> 
> Because the single quote escape is: '\''
> 
> $ echo ''\''hello world'\'''
> 'hello world'
> $ echo 'I'\''m a poor man'
> I'm a poor man

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?