zsh-workers
 help / color / mirror / code / Atom feed
* [BUG] Two vulnerabilities in zsh
@ 2020-05-19  6:48 Aaron Esau
  2020-05-19 17:04 ` Daniel Shahaf
  2020-05-20  0:40 ` [BUG] Two vulnerabilities in zsh - #1 :: null dereference in check_colon_subscript in subst.c Daniel Shahaf
  0 siblings, 2 replies; 9+ messages in thread
From: Aaron Esau @ 2020-05-19  6:48 UTC (permalink / raw)
  To: zsh-workers; +Cc: Peter Stephenson

Another win for AFL!

There are two vulnerabilities in zsh <5.8. I'll request a couple CVE IDs if that's okay.

I contacted Peter <p.w.stephenson@ntlworld.com> about these a few days ago. Neither have common, practical attack scenarios ("oh no! someone with a shell could pop a shell!"), so I'm disclosing them here.

I did a little analysis on each vulnerability. The more severe one appears to be some form of memory corruption, but it's decently complex, and I couldn't find the root cause. I think it'll take someone more experienced than me to find it. But, I was able to track down the null dereference bug. :)

Frankly, I'm not a C developer, but I think I at least know how to fix the null dereference vuln, so I added a section with a patch there.

Thanks for the awesome shell!

Sincerely,
Aaron Esau

https://aaronesau.com/


------ #1 :: null dereference in check_colon_subscript in subst.c ------


A denial of service vulnerability exists in zsh <5.8 due to a null dereference in check_colon_subscript in subst.c.

## Reproduction Steps

I was able to reproduce this bug on every zsh version I tested. I am running zsh 5.8 (x86_64-pc-linux-gnu) on Arch Linux.

1. Start zsh and execute the following (including quotes):

  "${: :${{{\"{{i use arch btw}}"

2. Observe that the command causes a segmentation fault. The stack trace is:

  $rip 0x5555555eb22e => movzx eax, BYTE PTR [rax]
  [#1] 0x5555555ed61e => prefork()
  [#2] 0x555555583815 => mov rsi, r12
  [#3] 0x555555588e01 => mov rbx, QWORD PTR [rbx]
  [#4] 0x55555558c32f => pop rcx
  [#5] 0x55555558c6d1 => mov eax, DWORD PTR [rip+0x9e8c1] # [rip+0x9e8c1] => 0x55555562af98
  [#6] 0x55555558e051 => execlist()
  [#7] 0x55555558e564 => execode()
  [#8] 0x5555555a43d4 => loop()
  [#9] 0x5555555a7d36 => zsh_main()

## Analysis

The following code in check_colon_subscript in subst.c checks if the value at a pointer obtained from a call to parse_subscript is NULL:

  *endp = parse_subscript(str, 0, ':');
  if (!*endp) {
      /* No trailing colon? */
      *endp = parse_subscript(str, 0, '\0');
      if (!*endp)
          return NULL;
  }

However, the pointer itself can be NULL. In parse_subscript in lex.c, if err != 0, the returning variable, s, is set to NULL:

  err = dquote_parse(endchar, sub);
  ...
  if (err) {
      ...
      s = NULL;
  } else {
      s += toklen;
  }
  ..
  return s;

The function dquote_parse in lex.c returns NULL on many error-related conditions.

## Patch

diff --git Src/subst.c Src/subst.c
index 90b5fc121..ac12c6d0e 100644
--- Src/subst.c
+++ Src/subst.c
@@ -1571,10 +1571,10 @@ check_colon_subscript(char *str, char **endp)
     }

     *endp = parse_subscript(str, 0, ':');
-    if (!*endp) {
+    if (endp && !*endp) {
 	/* No trailing colon? */
 	*endp = parse_subscript(str, 0, '\0');
-	if (!*endp)
+	if (endp && !*endp)
 	   return NULL;
     }
     sav = **endp;


------ #2 :: memory corruption in ?? ------


A memory corruption vulnerability exists in zsh <5.8 which may enable arbitrary code execution or memory disclosure via unspecified methods.

## Reproduction Steps

I am running zsh 5.8 (x86_64-pc-linux-gnu) on Arch Linux, kernel version 5.6.11-arch1-1.

1. Execute the following PoC command:

  echo $'******** **********************$\\\n(>$' | zsh

2. Observe that the command causes a segmentation fault. The stack trace is:

  $rip 0x5555555eb22e => movzx eax, BYTE PTR [rax]
  [#1] 0x5555555ed61e => prefork()
  [#2] 0x555555583815 => mov rsi, r12
  [#3] 0x555555588e01 => mov rbx, QWORD PTR [rbx]
  [#4] 0x55555558c32f => pop rcx
  [#5] 0x55555558c6d1 => mov eax, DWORD PTR [rip+0x9e8c1] # [rip+0x9e8c1] => 0x55555562af98
  [#6] 0x55555558e051 => execlist()
  [#7] 0x55555558e564 => execode()
  [#8] 0x5555555a43d4 => loop()
  [#9] 0x5555555a7d36 => zsh_main()

## Analysis

The segmentation fault is caused by a mov instruction which dereferences a $rax, a pointer to invalid memory:

  * 0x555555588bac: setne  bl
  * 0x555555588baf: jmp    0x555555588bd9
  * 0x555555588bb1: nop    DWORD PTR [rax+0x0]
  =>0x555555588bb8: mov    rdi, QWORD PTR [rax+0x10]
  * 0x555555588bbc: addr32 call 0x5555555f27f0 <has_token>
  * 0x555555588bc2: test   eax, eax
  * 0x555555588bc4: je     0x555555589bc0
  * 0x555555588bca: mov    rsi, QWORD PTR [rbp+0x0]
  * 0x555555588bce: xor    edx, edx

At the segfault, the \$r[a-z]{2} registers are:

  $rax : 0x00007fff007ffff7
  $rbx : 0x00007ffff7fbe601 => 0xa8772e2a3a000000
  $rcx : 0x00007ffff7fbe608 => 0x00007ffff7fbe5a8 => 0x0000000000000000
  $rdx : 0x00007ffff7fbe5a8 => 0x0000000000000000
  $rsp : 0x00007fffffffbef0 => 0x0000000100000099
  $rbp : 0x00007ffff7fbe5f0 => 0x00007fff007ffff7
  $rsi : 0x00007ffff7fbe578 => 0x0000000000000000
  $rdi : 0x00007ffff7fbe5f0 => 0x00007fff007ffff7
  $rip : 0x0000555555588bb8 => mov rdi, QWORD PTR [rax+0x10]

So, again, the register $rax points to invalid memory. I stepped backward and found that the value in the register comes from the stack:

  =>0x555555588bef: mov    rax, QWORD PTR [rbp+0x0]
  * 0x555555588bf3: test   rax, rax
  * 0x555555588bf6: jne    0x555555588bb8

I set a breakpoint at 0x555555588bef, then set watchpoints on both $rbp and—as least significant 4 bytes of the address appear to be overwritten—the address $rbp-0x4.

It was really hard to trace back and find what overwrites that memory. It appears as though the lower bytes of the address are overwritten before the instruction that first triggered the second watchpoint in __memmove_avx_unaligned_erms:

  =>0x7ffff7d4d26f: mov    QWORD PTR [rdi+rdx*1-0x8], rcx

However, neither execlist in exec.c nor any of the functions it calls appear to execute glibc's memmove function. Perhaps another glibc function internally calls __memmove_avx_unaligned_erms? To be continued.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [BUG] Two vulnerabilities in zsh
  2020-05-19  6:48 [BUG] Two vulnerabilities in zsh Aaron Esau
@ 2020-05-19 17:04 ` Daniel Shahaf
  2020-05-19 20:38   ` Peter Stephenson
  2020-05-20  0:40 ` [BUG] Two vulnerabilities in zsh - #1 :: null dereference in check_colon_subscript in subst.c Daniel Shahaf
  1 sibling, 1 reply; 9+ messages in thread
From: Daniel Shahaf @ 2020-05-19 17:04 UTC (permalink / raw)
  To: Aaron Esau; +Cc: zsh-workers, Peter Stephenson

tl;dr: Despite Mr Esau's subject line, I don't see any "vulnerabilities"
here.  Rather, all I see is two bugs that manifest as segfaults.  More
below.

Aaron Esau wrote on Tue, 19 May 2020 06:48 +0000:
> ------ #1 :: null dereference in check_colon_subscript in subst.c ------
> 
> 
> ## Reproduction Steps
> 
> I was able to reproduce this bug on every zsh version I tested. I am running zsh 5.8 (x86_64-pc-linux-gnu) on Arch Linux.
> 
> 1. Start zsh and execute the following (including quotes):
> 
>   "${: :${{{\"{{i use arch btw}}"
> 
> 2. Observe that the command causes a segmentation fault.

Confirmed in 5.7.1 and master.

> The stack trace is:
> 
>   $rip 0x5555555eb22e => movzx eax, BYTE PTR [rax]
>   [#1] 0x5555555ed61e => prefork()
>   [#2] 0x555555583815 => mov rsi, r12
>   [#3] 0x555555588e01 => mov rbx, QWORD PTR [rbx]
>   [#4] 0x55555558c32f => pop rcx
>   [#5] 0x55555558c6d1 => mov eax, DWORD PTR [rip+0x9e8c1] # [rip+0x9e8c1] => 0x55555562af98
>   [#6] 0x55555558e051 => execlist()
>   [#7] 0x55555558e564 => execode()
>   [#8] 0x5555555a43d4 => loop()
>   [#9] 0x5555555a7d36 => zsh_main()

> A denial of service vulnerability exists in zsh <5.8 due to a null dereference in check_colon_subscript in subst.c.

s/</≤/

More importantly, this is not a denial of service vulnerability.  For it
to qualify as a vulnerability, a situation would have to be identified
in which someone who _doesn't_ control the source code being executed
can trigger the segfault.  Until such a case is identified, this is
merely a bug that manifests as a segfault.

We thank you for the reproduction recipe and the patch.  We'll add
a regression test based on the recipe and review the patch.  Right now,
however, I consider it a higher priority to finish assessing your other
bug report and post my conclusions of both reports.

> ------ #2 :: memory corruption in ?? ------
> 
> 
> ## Reproduction Steps
> 
> I am running zsh 5.8 (x86_64-pc-linux-gnu) on Arch Linux, kernel version 5.6.11-arch1-1.
> 
> 1. Execute the following PoC command:
> 
>   echo $'******** **********************$\\\n(>$' | zsh

This instruction is underspecified because it does not identify «echo»
implementation being used and the shell being used (which affects how
the «$'…'» would be parsed).  That aside, I can reproduce this:

$ printf '******** **********************$\\\n(>$' | zsh -f 
 BUG: parse error in command substitution
Segmentation fault
$ 

That's zsh from master.  The BUG message is likely generated in debug
builds only.

> A memory corruption vulnerability exists in zsh <5.8 which may enable arbitrary code execution or memory disclosure via unspecified methods.

Everything I wrote about #1 applies here, too.  In particular, this
isn't a vulnerability either.

> There are two vulnerabilities in zsh <5.8. I'll request a couple CVE IDs if that's okay.
> 

Assignment of a CVE ID does not constitute recognition of a report as
a vulnerability.  The purpose of CVE ID's is not to catalogue _valid_
issues but to catalogue _potential_ issues; to give them names so they
may be referred to unambiguously.  On that basis, assigning CVE ID's to
_suspected_ issues while they're under discussion is perfectly normal.

However, to avoid any possibility of misunderstanding the previous
paragraph, let me state my view unambiguously:

    I am a zsh developer and I do not consider these to be vulnerabilities,
    since triggering the segfaults seems to require the attacker to be
    able to control the script's source code.

You can also refer to these issues using the X-Seq number of your
message.  In this case, this would be "45853#1" and "45853#2", since
"workers/45853" is the X-Seq identifier of your report:
http://www.zsh.org/cgi-bin/mla/redirect?WORKERNUMBER=45843

> I contacted Peter <p.w.stephenson@ntlworld.com> about these a few days ago. Neither have common, practical attack scenarios ("oh no! someone with a shell could pop a shell!"), so I'm disclosing them here.
> 

You didn't disclose vulnerabilities; you reported bugs.  If they had
been vulnerabilities, they would not have been made public before
a patch had been prepared and vetted.

The bug reports are valid and appreciated, and will be fixed.  However,
your subject line was inappropriate: it portrayed you as a boy crying
"Wolf!" and required me to rearrange my morning at short notice.

To the list, let's please not discuss #1 and #2 under the same subject
line as each other.

> I did a little analysis on each vulnerability. The more severe one appears to be some form of memory corruption, but it's decently complex, and I couldn't find the root cause. I think it'll take someone more experienced than me to find it. But, I was able to track down the null dereference bug. :)
> 
> Frankly, I'm not a C developer, but I think I at least know how to fix the null dereference vuln, so I added a section with a patch there.
> 
> Thanks for the awesome shell!
> 
> Sincerely,
> Aaron Esau

For future reference, acrostics aren't customarily embedded in
vulnerability reports.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [BUG] Two vulnerabilities in zsh
  2020-05-19 17:04 ` Daniel Shahaf
@ 2020-05-19 20:38   ` Peter Stephenson
  2020-05-20  0:45     ` Daniel Shahaf
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Stephenson @ 2020-05-19 20:38 UTC (permalink / raw)
  To: zsh-workers

On Tue, 2020-05-19 at 17:04 +0000, Daniel Shahaf wrote:
> > 1. Execute the following PoC command:
> > 
> >   echo $'******** **********************$\\\n(>$' | zsh
> 
> This instruction is underspecified because it does not identify «echo»
> implementation being used and the shell being used (which affects how
> the «$'…'» would be parsed).  That aside, I can reproduce this:
> 
> $ printf '******** **********************$\\\n(>$' | zsh -f 
>  BUG: parse error in command substitution
> Segmentation fault
> $ 

The BUG message simplifies to this:

(127)9:32% zsh -fc '$\
('
1: BUG: parse error in command substitution
zsh:1: no such file or directory: pws/.

The other output shows it's doing something it shouldn't even if there
isn't a crash as a result.  Adding a command in front does produce a
crash.

I think the backslashed newline is valid, and it looks like it's usually
correctly handled; apparently its presence is disguising the bad input
in this case.

pws


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [BUG] Two vulnerabilities in zsh - #1 :: null dereference in check_colon_subscript in subst.c
  2020-05-19  6:48 [BUG] Two vulnerabilities in zsh Aaron Esau
  2020-05-19 17:04 ` Daniel Shahaf
@ 2020-05-20  0:40 ` Daniel Shahaf
  1 sibling, 0 replies; 9+ messages in thread
From: Daniel Shahaf @ 2020-05-20  0:40 UTC (permalink / raw)
  To: Aaron Esau; +Cc: zsh-workers, Peter Stephenson

Aaron Esau wrote on Tue, 19 May 2020 06:48 +0000:
> The following code in check_colon_subscript in subst.c checks if the value at a pointer obtained from a call to parse_subscript is NULL:
> 
>   *endp = parse_subscript(str, 0, ':');
>   if (!*endp) {
>       /* No trailing colon? */
>       *endp = parse_subscript(str, 0, '\0');
>       if (!*endp)
>           return NULL;
>   }  
> 
> However, the pointer itself can be NULL. [...]
> 
> ## Patch
> 
> diff --git Src/subst.c Src/subst.c
> index 90b5fc121..ac12c6d0e 100644
> --- Src/subst.c
> +++ Src/subst.c
> @@ -1571,10 +1571,10 @@ check_colon_subscript(char *str, char **endp)
>      }  
> 

Your MUA corrupted the patch by munging the trailing whitespace.

You generated the patch without the a/ and b/ prefixes.  I tried this
once but discovered (to my dismay) that «git apply» doesn't apply such
patches by default.  How do you deal with that?

>      *endp = parse_subscript(str, 0, ':');
> -    if (!*endp) {
> +    if (endp && !*endp) {
>  	/* No trailing colon? */
>  	*endp = parse_subscript(str, 0, '\0');
> -	if (!*endp)
> +	if (endp && !*endp)
>  	   return NULL;
>      }  
>      sav = **endp;

endp has already been dereferenced by the time you have it tested for
NULL.  Thus, this patch is a no-op.  In fact, you'll probably find the
(optimized) object code generated for this snippet is not changed by
applying this patch.  If this patch changes the behaviour at all, that'd
be a compiler bug.

Besides, all callers pass non-NULL values for endp.

Did you post the correct patch?

Daniel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [BUG] Two vulnerabilities in zsh
  2020-05-19 20:38   ` Peter Stephenson
@ 2020-05-20  0:45     ` Daniel Shahaf
  2020-05-22 18:56       ` Peter Stephenson
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Shahaf @ 2020-05-20  0:45 UTC (permalink / raw)
  To: zsh-workers; +Cc: Aaron Esau

Peter Stephenson wrote on Tue, 19 May 2020 21:38 +0100:
> On Tue, 2020-05-19 at 17:04 +0000, Daniel Shahaf wrote:
> > > 1. Execute the following PoC command:
> > > 
> > >   echo $'******** **********************$\\\n(>$' | zsh  
> > 
> > This instruction is underspecified because it does not identify «echo»
> > implementation being used and the shell being used (which affects how
> > the «$'…'» would be parsed).  That aside, I can reproduce this:
> > 
> > $ printf '******** **********************$\\\n(>$' | zsh -f 
> >  BUG: parse error in command substitution
> > Segmentation fault
> > $   
> 
> The BUG message simplifies to this:
> 
> (127)9:32% zsh -fc '$\
> ('
> 1: BUG: parse error in command substitution
> zsh:1: no such file or directory: pws/.
> 
> The other output shows it's doing something it shouldn't even if there
> isn't a crash as a result.  Adding a command in front does produce a
> crash.
> 
> I think the backslashed newline is valid, and it looks like it's usually
> correctly handled; apparently its presence is disguising the bad input
> in this case.

Test cases:

diff --git a/Test/D04parameter.ztst b/Test/D04parameter.ztst
index 024de4d2b..6d2dd0d99 100644
--- a/Test/D04parameter.ztst
+++ b/Test/D04parameter.ztst
@@ -2656,3 +2656,13 @@ F:behavior, see http://austingroupbugs.net/view.php?id=888
 >1: pws
 >3: pw
 >4: pw
+
+ # Using a subshell because it segfaults.
+ ("${: :${{{\"{{lorem ipsum dolor sit amet}}")
+-f:regression test for workers/45843#1
+?(eval):1: bad substitution
+
+# Temporarily using the 'D' flag because it generates a "BUG:" message in
+# debug builds only.
+ $ZTST_testdir/../Src/zsh -fc $'$\\\n('
+1Df:regression test for workers/45843#2: escaped newline in command substitution start token

I haven't added the crashing version of 45843#2.  Let me know if I should.

Cheers,

Daniel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [BUG] Two vulnerabilities in zsh
  2020-05-20  0:45     ` Daniel Shahaf
@ 2020-05-22 18:56       ` Peter Stephenson
  2020-05-22 21:47         ` Peter Stephenson
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Stephenson @ 2020-05-22 18:56 UTC (permalink / raw)
  To: zsh-workers

On Wed, 2020-05-20 at 00:45 +0000, Daniel Shahaf wrote:
> Peter Stephenson wrote on Tue, 19 May 2020 21:38 +0100:
> > The BUG message simplifies to this:
> > 
> > (127)9:32% zsh -fc '$\
> > ('
> > 1: BUG: parse error in command substitution
> > zsh:1: no such file or directory: pws/.
> > 
> > The other output shows it's doing something it shouldn't even if there
> > isn't a crash as a result.  Adding a command in front does produce a
> > crash.
> > 
> > I think the backslashed newline is valid, and it looks like it's usually
> > correctly handled; apparently its presence is disguising the bad input
> > in this case.
> 
> Test cases:

This seems to be doing the right thing.  It's a bit hairy, but I think
it just *is* a bit hairy --- we do need to re-input the multi-character
special sequence if there's something to ignore in the middle.  I can't
see an obvious problem with how I've done it as it can't be triggered by
anythign else that I can see.

There may be other sequences like this.

I believe inlined patches work from this version of Evolution...

pws

diff --git a/Src/lex.c b/Src/lex.c
index a541defe6..5382a5309 100644
--- a/Src/lex.c
+++ b/Src/lex.c
@@ -1237,6 +1237,19 @@ gettokstr(int c, int sub)
 	case LX2_BKSLASH:
 	    c = hgetc();
 	    if (c == '\n') {
+		if (lexbuf.len && !lexstop &&
+		    (lexbuf.ptr[-1] == String ||
+		     lexbuf.ptr[-1] == Qstring))
+		{
+		    /*
+		     * $-prefixed expression interrupted by \\\n.
+		     * This is valid --- reparse.
+		     */
+		    --lexbuf.len;
+		    --lexbuf.ptr;
+		    c = '$';
+		    continue;
+		}
 		c = hgetc();
 		if (!lexstop)
 		    continue;
diff --git a/Test/D04parameter.ztst b/Test/D04parameter.ztst
index 6d2dd0d99..a3661f5da 100644
--- a/Test/D04parameter.ztst
+++ b/Test/D04parameter.ztst
@@ -2662,7 +2662,6 @@ F:behavior, see http://austingroupbugs.net/view.php?id=888
 -f:regression test for workers/45843#1
 ?(eval):1: bad substitution
 
-# Temporarily using the 'D' flag because it generates a "BUG:" message in
-# debug builds only.
  $ZTST_testdir/../Src/zsh -fc $'$\\\n('
-1Df:regression test for workers/45843#2: escaped newline in command substitution start token
+1f:regression test for workers/45843#2: escaped newline in command substitution start token
+?(eval):1: parse error near `$('


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [BUG] Two vulnerabilities in zsh
  2020-05-22 18:56       ` Peter Stephenson
@ 2020-05-22 21:47         ` Peter Stephenson
  2020-05-23  2:17           ` Daniel Shahaf
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Stephenson @ 2020-05-22 21:47 UTC (permalink / raw)
  To: zsh-workers

On Fri, 2020-05-22 at 19:56 +0100, Peter Stephenson wrote:
> On Wed, 2020-05-20 at 00:45 +0000, Daniel Shahaf wrote:
> > Peter Stephenson wrote on Tue, 19 May 2020 21:38 +0100:
> > > The BUG message simplifies to this:
> > > 
> > > (127)9:32% zsh -fc '$\
> > > ('
> > > 1: BUG: parse error in command substitution
> > > zsh:1: no such file or directory: pws/.
> > > 
> > > The other output shows it's doing something it shouldn't even if there
> > > isn't a crash as a result.  Adding a command in front does produce a
> > > crash.
> > > 
> > > I think the backslashed newline is valid, and it looks like it's usually
> > > correctly handled; apparently its presence is disguising the bad input
> > > in this case.
> > 
> > Test cases:
> 
> This seems to be doing the right thing.

Enhanced version: this handles the bizarre but valid

echo $(\
(3+2))

and adds a few tests for cases without a parse error.

pws

diff --git a/Src/lex.c b/Src/lex.c
index a541defe6..9d29c1bc1 100644
--- a/Src/lex.c
+++ b/Src/lex.c
@@ -541,6 +541,16 @@ cmd_or_math_sub(void)
 {
     int c = hgetc(), ret;
 
+    if (c == '\\') {
+	c = hgetc();
+	if (c != '\n') {
+	    hungetc(c);
+	    hungetc('\\');
+	    return skipcomm() ? CMD_OR_MATH_ERR : CMD_OR_MATH_CMD;
+	}
+	c = hgetc();
+    }
+
     if (c == '(') {
 	int lexpos = (int)(lexbuf.ptr - tokstr);
 	add(Inpar);
@@ -1237,6 +1247,19 @@ gettokstr(int c, int sub)
 	case LX2_BKSLASH:
 	    c = hgetc();
 	    if (c == '\n') {
+		if (lexbuf.len && !lexstop &&
+		    (lexbuf.ptr[-1] == String ||
+		     lexbuf.ptr[-1] == Qstring))
+		{
+		    /*
+		     * $-prefixed expression interrupted by \\\n.
+		     * This is valid --- reparse.
+		     */
+		    --lexbuf.len;
+		    --lexbuf.ptr;
+		    c = '$';
+		    continue;
+		}
 		c = hgetc();
 		if (!lexstop)
 		    continue;
diff --git a/Test/D04parameter.ztst b/Test/D04parameter.ztst
index 6d2dd0d99..b0b53c7d0 100644
--- a/Test/D04parameter.ztst
+++ b/Test/D04parameter.ztst
@@ -2662,7 +2662,25 @@ F:behavior, see http://austingroupbugs.net/view.php?id=888
 -f:regression test for workers/45843#1
 ?(eval):1: bad substitution
 
-# Temporarily using the 'D' flag because it generates a "BUG:" message in
-# debug builds only.
  $ZTST_testdir/../Src/zsh -fc $'$\\\n('
-1Df:regression test for workers/45843#2: escaped newline in command substitution start token
+1f:regression test for workers/45843#2: escaped newline in command substitution start token
+?(eval):1: parse error near `$('
+
+# `
+
+ eval $'echo $\\\n(printf "%d\\n" $(( 4 + 2 )) )'
+0:Normal command substitution with escaped newline
+>6
+
+ eval $'echo $\\\n(( 14 / 2 ))'
+0:Normal math eval with escaped newline after $
+>7
+
+ eval $'echo $(\\\n( 15 / 3 ))'
+0:Normal math eval with escaped newline after $(
+>5
+
+  function '*' { echo What a star; }
+  eval 'echo $(\*)'
+0:Backslash character other than newline is normal after $(
+>What a star


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [BUG] Two vulnerabilities in zsh
  2020-05-22 21:47         ` Peter Stephenson
@ 2020-05-23  2:17           ` Daniel Shahaf
  2020-05-23 16:45             ` Peter Stephenson
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Shahaf @ 2020-05-23  2:17 UTC (permalink / raw)
  To: zsh-workers

Peter Stephenson wrote on Fri, 22 May 2020 19:56 +0100:
> I believe inlined patches work from this version of Evolution...

Yes, they applied fine.  Thanks.  More below.

Peter Stephenson wrote on Fri, 22 May 2020 22:47 +0100:
> +++ b/Test/D04parameter.ztst
> @@ -2662,7 +2662,25 @@ F:behavior, see http://austingroupbugs.net/view.php?id=888
> -# Temporarily using the 'D' flag because it generates a "BUG:" message in
> -# debug builds only.
>   $ZTST_testdir/../Src/zsh -fc $'$\\\n('
> -1Df:regression test for workers/45843#2: escaped newline in command substitution start token
> +1f:regression test for workers/45843#2: escaped newline in command substitution start token
> +?(eval):1: parse error near `$('
> +

Note that you haven't removed the 'f' flag, which implies that this
test's $?/output/errput don't match the expected ones.  The 'f' flag
should be removed and the expected errput updated:

-(eval):1: parse error near `$('
+zsh:2: parse error near `$('

(That's copied from the output of «make check» after removing the 'f' flag.)

> + eval $'echo $\\\n(printf "%d\\n" $(( 4 + 2 )) )'
> +0:Normal command substitution with escaped newline
> +>6  

This test fails on my machine:

--- /tmp/zsh.ztst.24422/ztst.out        2020-05-23 02:11:04.239080323 +0000                                                                                                                                         
+++ /tmp/zsh.ztst.24422/ztst.tout       2020-05-23 02:11:04.239080323 +0000
@@ -1 +1 @@
-6  
+6
Test /home/daniel/src/zsh/./Test/D04parameter.ztst failed: output differs from expected as shown above for:
 eval $'echo $\\\n(printf "%d\\n" $(( 4 + 2 )) )'
Was testing: Normal command substitution with escaped newline

The difference is two trailing spaces in the expected output that are missing from the actual output.

The other new tests seem to have the same issue.

Thanks,

Daniel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [BUG] Two vulnerabilities in zsh
  2020-05-23  2:17           ` Daniel Shahaf
@ 2020-05-23 16:45             ` Peter Stephenson
  0 siblings, 0 replies; 9+ messages in thread
From: Peter Stephenson @ 2020-05-23 16:45 UTC (permalink / raw)
  To: zsh-workers

Change of plan: I've done the fix for the original problem the same way
I did the fix for the follow up.  That's because I can no longer convince
myself that a String token last in the buffer is necessarily the very
start of a $-expansion --- it may be true, I'm just not sure --- and we
might as well be consistent in the two case.

The reason I did it that way to begin with was that way back when we
could only back up one input character.  But that hasn't been true since
I rewrote the input layer (between specific I/O types and history) some
time ago now.

The test cases are fixed --- there aren't any trailing spaces, even if
they show up in the patch.

pws


diff --git a/Src/lex.c b/Src/lex.c
index a541defe6..37fcec3e2 100644
--- a/Src/lex.c
+++ b/Src/lex.c
@@ -541,6 +541,17 @@ cmd_or_math_sub(void)
 {
     int c = hgetc(), ret;
 
+    if (c == '\\') {
+	c = hgetc();
+	if (c != '\n') {
+	    hungetc(c);
+	    hungetc('\\');
+	    lexstop = 0;
+	    return skipcomm() ? CMD_OR_MATH_ERR : CMD_OR_MATH_CMD;
+	}
+	c = hgetc();
+    }
+
     if (c == '(') {
 	int lexpos = (int)(lexbuf.ptr - tokstr);
 	add(Inpar);
@@ -998,6 +1009,16 @@ gettokstr(int c, int sub)
 	    break;
 	case LX2_STRING:
 	    e = hgetc();
+	    if (e == '\\') {
+		e = hgetc();
+		if (e != '\n') {
+		    hungetc(e);
+		    hungetc('\\');
+		    lexstop = 0;
+		    break;
+		}
+		e = hgetc();
+	    }
 	    if (e == '[') {
 		cmdpush(CS_MATHSUBST);
 		add(String);
diff --git a/Test/D04parameter.ztst b/Test/D04parameter.ztst
index 6d2dd0d99..e51c955ee 100644
--- a/Test/D04parameter.ztst
+++ b/Test/D04parameter.ztst
@@ -2662,7 +2662,25 @@ F:behavior, see http://austingroupbugs.net/view.php?id=888
 -f:regression test for workers/45843#1
 ?(eval):1: bad substitution
 
-# Temporarily using the 'D' flag because it generates a "BUG:" message in
-# debug builds only.
  $ZTST_testdir/../Src/zsh -fc $'$\\\n('
-1Df:regression test for workers/45843#2: escaped newline in command substitution start token
+1:regression test for workers/45843#2: escaped newline in command substitution start token
+?zsh:2: parse error near `$('
+
+# `
+
+ eval $'echo $\\\n(printf "%d\\n" $(( 4 + 2 )) )'
+0:Normal command substitution with escaped newline
+>6
+
+ eval $'echo $\\\n(( 14 / 2 ))'
+0:Normal math eval with escaped newline after $
+>7
+
+ eval $'echo $(\\\n( 15 / 3 ))'
+0:Normal math eval with escaped newline after $(
+>5
+
+  function '*' { echo What a star; }
+  eval 'echo $(\*)'
+0:Backslash character other than newline is normal after $(
+>What a star


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-05-23 16:46 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-19  6:48 [BUG] Two vulnerabilities in zsh Aaron Esau
2020-05-19 17:04 ` Daniel Shahaf
2020-05-19 20:38   ` Peter Stephenson
2020-05-20  0:45     ` Daniel Shahaf
2020-05-22 18:56       ` Peter Stephenson
2020-05-22 21:47         ` Peter Stephenson
2020-05-23  2:17           ` Daniel Shahaf
2020-05-23 16:45             ` Peter Stephenson
2020-05-20  0:40 ` [BUG] Two vulnerabilities in zsh - #1 :: null dereference in check_colon_subscript in subst.c Daniel Shahaf

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).