GNU bug report logs - #59817
[PATCH] Fix etags local command injection vulnerability

Previous Next

Full log

Message #38 received at 59817 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: lux <lx <at> shellcodes.org>
Cc: stefankangas <at> gmail.com, 59817 <at> debbugs.gnu.org
Subject: Re: bug#59817: [PATCH] Fix etags local command injection vulnerability
Date: Tue, 06 Dec 2022 16:52:40 +0200

> Date: Tue, 6 Dec 2022 21:11:35 +0800
> From: lux <lx <at> shellcodes.org>
> Cc: stefankangas <at> gmail.com, 59817 <at> debbugs.gnu.org
> 
> On Tue, 06 Dec 2022 14:55:09 +0200
> Eli Zaretskii <eliz <at> gnu.org> 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?

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.