[BUG] Zsh crashes when using autocomplete because of memory unsafety (double free)

[BUG] Zsh crashes when using autocomplete because of memory unsafety (double free)

[Petr Šťastný]

[-- Attachment #1.1: Type: text/plain, Size: 1156 bytes --]

Note: I'm using ohmyzsh to provide autocomplete and I can't reproduce it
without ohmyzsh, but since ohmyzsh is written entirely in shell, this
should be an issue in zsh itself.

When I trigger autocomplete in one position, zsh crashes, yielding one
of the following messages, seemingly at random:

(none, zsh crashes silently)         x12
double free or corruption (out)     x5

Zsh crashed each time I tried it.

zsh 5.8 (x86_64-pc-linux-gnu)
ohmyzsh, commit 6152ac30bede172ba0422a8610dc796948ae1546

Minimal setup:
$ alias a='""'
           ^

Place cursor so it stands on the first " character and press Tab to
trigger autocomplete. Zsh crashes.

I have almost default ohmyzsh installation, the only thing I changed is
theme (to `mrtazz`) and `git` plugin, none of which should affect this.
Zsh config itself (after being modified by ohmyzsh) is unchanged, if one
doesn't count `PATH` exports. Thus I don't see the need to attach my
full config. If anyone has troubles reproducing this, let me know. But
it should be enough to have ohmyzsh installed.

-- 
Petr Šťastný
FIT ČVUT

soptik.tech



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [BUG] Zsh crashes when using autocomplete because of memory unsafety (double free)

[Daniel Shahaf]

Petr Šťastný wrote on Fri, 26 Jun 2020 17:03 +0200:
> Note: I'm using ohmyzsh to provide autocomplete and I can't reproduce it
> without ohmyzsh, but since ohmyzsh is written entirely in shell, this
> should be an issue in zsh itself.
>
> When I trigger autocomplete in one position, zsh crashes, yielding one
> of the following messages, seemingly at random:
>
> (none, zsh crashes silently)         x12
> double free or corruption (out)     x5
>
> Zsh crashed each time I tried it.
>
> zsh 5.8 (x86_64-pc-linux-gnu)
> ohmyzsh, commit 6152ac30bede172ba0422a8610dc796948ae1546
>
> Minimal setup:
> $ alias a='""'
>            ^
>
> Place cursor so it stands on the first " character and press Tab to
> trigger autocomplete. Zsh crashes.
>
> I have almost default ohmyzsh installation, the only thing I changed is
> theme (to `mrtazz`) and `git` plugin, none of which should affect this.
> Zsh config itself (after being modified by ohmyzsh) is unchanged, if one
> doesn't count `PATH` exports. Thus I don't see the need to attach my
> full config. If anyone has troubles reproducing this, let me know. But
> it should be enough to have ohmyzsh installed.
>

tl;dr: I can reproduce two different segfaults here: one of them with
a minimal reproduction recipe; one with my personal setup, and
a specific setopt makes it go away.

With personal setup with current master:

[[[
% alias a='""'<Left><Left><Left><TAB>
16: ./Src/Zle/compcore.c:1678: expecting 'x' at offset -1 of "x"""
Process 378 stopped
* thread #1, name = 'zsh', stop reason = signal SIGSEGV: invalid address (fault address: 0x7ffff6c84fff)
    frame #0: 0x000000000053046e zsh`check_param(s="\x9e\x9e", set=0, test=1) at compcore.c:1127
   1124      * offset "offs" into it via a global sucks badly.
   1125      */
   1126     for (p = s + offs; ; p--) {
-> 1127         if (*p == String || *p == Qstring) {
   1128             /*
   1129              * String followed by Snull (unquoted) or
   1130              * QString followed by ' (quoted) indicate a nested
(lldb) bt 4
* thread #1, name = 'zsh', stop reason = signal SIGSEGV: invalid address (fault address: 0x7ffff6c84fff)
  * frame #0: 0x000000000053046e zsh`check_param(s="\x9e\x9e", set=0, test=1) at compcore.c:1127
    frame #1: 0x0000000000531c70 zsh`set_comp_sep at compcore.c:1698
    frame #2: 0x000000000052dbba zsh`bin_compset(name="compset", argv=0x00007ffff6c85958, ops=0x00007ffffffd2358, func=0) at complete.c:1140
    frame #3: 0x00000000004216e1 zsh`execbuiltin(args=0x00007ffff6c85928, assigns=0x0000000000000000, bn=0x00000000005e7c60) at builtin.c:507
(lldb) p funcstack[0]
(funcstack) $0 = {
  prev = 0x00007ffffffdbe78
  name = 0x00007ffff6c85160 "_alias"
  filename = 0x00007ffff6c85168 "/srv/zsh/share/zsh/5.8.0.2-dev/functions/_alias"
  caller = 0x00000000005a33b3 "(eval)"
  flineno = 0
  lineno = 1
  tp = 1
}
(lldb) p funcstack[1]
(funcstack) $1 = {
  prev = 0x00007361696c615f
  name = 0x68737a2f7672732f <no value available>
  filename = 0x7a2f65726168732f <no value available>
  caller = 0x302e382e352f6873 <no value available>
  flineno = 7363234093618508334
  lineno = 8317708060514807413
  tp = 1818320687
}
(lldb) p s
(char *) $2 = 0x00007ffff6c85a10 "\x9e\x9e"
(lldb) p offs
(int) $3 = -1
]]]

In frame #0's actual arguments, 0x9e is Dnull.

funcstack[1] appears to be a memory block comprising some strings
(reversed because I'm on a little-endian platform):
.
    % () { for 1; print -r -- $1 "$(xxd -p -r <<<${1#0x})" } 0x00007361696c615f 0x68737a2f7672732f 0x7a2f65726168732f 0x302e382e352f6873
    0x00007361696c615f saila_
    0x68737a2f7672732f hsz/vrs/
    0x7a2f65726168732f z/erahs/
    0x302e382e352f6873 0.8.5/hs
    % xxd -p -r <<<$(printf %x 7363234093618508334); echo
    f/ved-2.
    % xxd -p -r <<<$(printf %x 8317708060514807413); echo
    snoitcnu
    % xxd -p -r <<<$(printf %x 1818320687); echo
    la_/
    (lldb) p (char*) &funcstack[1].filename
    (char *) $0 = 0x00007ffff6c85170 "/share/zsh/5.8.0.2-dev/functions/_alias"

The segfault goes away when I unsetopt completeinword.

From that, I get a minimal segfault with a different backtrace:

[[[
$ lldb -- /srv/zsh/bin/zsh -f
% autoload compinit
% compinit
% setopt completeinword
% alias a='""'<Left><Left><Left><TAB>
16: ./Src/Zle/compcore.c:1678: expecting 'x' at offset -1 of "x"""
Process 3116 stopped
* thread #1, name = 'zsh', stop reason = signal SIGSEGV: invalid address (fault address: 0x0)
    frame #0: 0x00007ffff7c74df2 libc.so.6`malloc_consolidate(av=0x00007ffff7db0c40) at malloc.c:4494
(lldb) bt 7
* thread #1, name = 'zsh', stop reason = signal SIGSEGV: invalid address (fault address: 0x0)
  * frame #0: 0x00007ffff7c74df2 libc.so.6`malloc_consolidate(av=0x00007ffff7db0c40) at malloc.c:4494
    frame #1: 0x00007ffff7c77a58 libc.so.6`_int_malloc(av=0x00007ffff7db0c40, bytes=2036) at malloc.c:3695
    frame #2: 0x00007ffff7c793e3 libc.so.6`__GI___libc_malloc(bytes=2036) at malloc.c:3049
    frame #3: 0x000000000048d49d zsh`zalloc(size=2036) at mem.c:966
    frame #4: 0x00000000004492ff zsh`getfpfunc(s="_command_names", ksh=0x00007ffffffd0f1c, fdir=0x00007ffffffd0f08, alt_path=0x0000000000000000, test_only=0) at exec.c:6097
    frame #5: 0x0000000000448d96 zsh`loadautofn(shf=0x000000000069c790, fksh=1, autol=0, current_fpath=0) at exec.c:5582
    frame #6: 0x000000000044ef7c zsh`execcmd_exec(state=0x00007ffffffd2b20, eparams=0x00007ffffffd1ae8, input=0, output=0, how=18, last1=2, close_if_forked=-1) at exec.c:3433
(lldb) p funcstack[0]
(funcstack) $0 = {
  prev = 0x00007ffff6cd0128
  name = 0x00007ffff6ccc160 "_command_names"
  filename = 0x0000000000000000 <no value available>
  caller = 0x00007ffff6cd0160 "_autocd"
  flineno = 0
  lineno = 3
  tp = 1
}
(lldb) p funcstack[1]
(funcstack) $1 = {
  prev = 0x646e616d6d6f635f
  name = 0x000073656d616e5f <no value available>
  filename = 0x636f6c2f7273752f <no value available>
  caller = 0x65726168732f6c61 <no value available>
  flineno = 8388362428407314991
  lineno = 7598807797348052325
  tp = 779316847
}
(lldb) 
]]]

    % () { for 1; print -r -- $1 "$(xxd -p -r <<<${1#0x})" } 0x646e616d6d6f635f 0x000073656d616e5f 0x636f6c2f7273752f 0x65726168732f6c61
    0x646e616d6d6f635f dnammoc_
    0x000073656d616e5f seman_
    0x636f6c2f7273752f col/rsu/
    0x65726168732f6c61 erahs/la
    % for 1 in 8388362428407314991 7598807797348052325 779316847 ; { xxd -p -r <<<$(printf %x $1) | xxd }
    00000000: 7469 732f 6873 7a2f                      tis/hsz/
    00000000: 6974 636e 7566 2d65                      itcnuf-e
    00000000: 2e73 6e6f                                .sno
    (lldb) p (char*)&funcstack[1].filename
    (char *) $6 = 0x00007ffff6ccc170 "/usr/local/share/zsh/site-functions.zwc"

I can reproduce this latter variant in 5.7.1 as well.  I haven't tried
older versions.

I haven't tried reproducing the issue under ohmyzsh.  It could in
theory be a different one.

Petr, could you please check whether the issue reproduces if you
«unsetopt completeinword»?

Cheers,

Daniel

Re: [BUG] Zsh crashes when using autocomplete because of memory unsafety (double free)

[Roman Perepelitsa]

On Fri, Jun 26, 2020 at 6:24 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> % alias a='""'<Left><Left><Left><TAB>
> 16: ./Src/Zle/compcore.c:1678: expecting 'x' at offset -1 of "x"""

(digression)

Does lldb start automatically when zsh crashes on you machine? How did
you configure it this way?

Roman.

zsh/lldb (was: Re: [BUG] Zsh crashes when using autocomplete because of memory unsafety (double free))

[Daniel Shahaf]

Roman Perepelitsa wrote on Fri, 26 Jun 2020 18:32 +0200:
> On Fri, Jun 26, 2020 at 6:24 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> >
> > % alias a='""'<Left><Left><Left><TAB>
> > 16: ./Src/Zle/compcore.c:1678: expecting 'x' at offset -1 of "x"""  
> 
> (digression)
> 
> Does lldb start automatically when zsh crashes on you machine?

No, it doesn't.  I ran «lldb /srv/zsh/bin/zsh» and typed «r» to launch
zsh, and trimmed that part of the output before posting.  I also trimmed
the value of $abs_srcdir and a few other things.

And in any case, the message above is a DPUTS(); it has nothing to do
with whether zsh was run under a debugger or not.

Although, come to think of it, I'm not sure where that "16:" at the
start of the line is from.

> How did you configure it this way?

Cheers,

Daniel

P.S.  Double digression: How do I invoke lldb in a way that'll cause it
to run the target automatically?  I'm looking for something along the
lines of «lldb --foo r -- /path/to/zsh», where «r» is the lldb command
for starting the target.  I've tried -o and -s and they do start zsh,
but the tty settings are wrong (^D doesn't work, for example).

Re: [BUG] Zsh crashes when using autocomplete because of memory unsafety (double free)

[Bart Schaefer]

[-- Attachment #1: Type: text/plain, Size: 926 bytes --]

On Fri, Jun 26, 2020 at 9:24 AM Daniel Shahaf <d.s@daniel.shahaf.name>
wrote:

>
> tl;dr: I can reproduce two different segfaults here: one of them with
> a minimal reproduction recipe; one with my personal setup, and
> a specific setopt makes it go away.
>

Even more minimal:

% autoload compinit
% compinit -D
% setopt completeinword
% alias a='"<left><TAB>

I think it has to do with
compset -P 1 '*='
compset -q

If you look at what happens with

% alias a='<TAB>
% alias a=

Alternately,

% alias a='"<TAB>
% alias a=\"

So, what's happening is that a=' turns into a=\" and consequently increases
the offset by one, but then when completion fails (because of the
double-quote not matching anything to the right of the cursor) and the
original command line is restored, the new larger offset is applied to the
original a=' string and indexes off the left end.

Having gotten that far, though, I don't know how to fix it.

Re: [BUG] Zsh crashes when using autocomplete because of memory unsafety (double free)

[Bart Schaefer]

[-- Attachment #1: Type: text/plain, Size: 3025 bytes --]

On Sun, Jun 28, 2020 at 12:09 AM Bart Schaefer <schaefer@brasslantern.com>
wrote:

>
> % autoload compinit
> % compinit -D
> % setopt completeinword
> % alias a='"<left><TAB>
>
> I think it has to do with
> compset -P 1 '*='
> compset -q
>
> So, what's happening is that a=' turns into a=\"
>

Just to clarify, you can actually watch this happening in gdb if you set a
watchpoint on "offs" and step through a few instructions.

toltec-ubuntu% alias a='"
Hardware watchpoint 1: offs

Old value = 3
New value = 2
get_comp_string () at zle_tricky.c:1883
1883    if (*p == Snull && isset(RCQUOTES))
(gdb) p p
$24 = 0x865592 "\235\""
(gdb) p zlemetaline
$25 = 0x8b7a40 "alias a='\""
(gdb) n
1885    if (p[1] || *p != Bnull) {
(gdb) n
1886 if (*p == Bnull) {
(gdb)
1890    ocs = zlemetacs;
(gdb)
1891    zlemetacs = i;
(gdb) p ocs
$26 = 9
(gdb) n
1892    foredel(skipchars, CUT_RAW);
(gdb) n
1893    if ((zlemetacs = ocs) > --i) {
(gdb) p skipchars
$27 = 1
(gdb) p zlemetaline
$28 = 0x8b7a40 "alias a=\""
(gdb) where 2
#0  get_comp_string () at zle_tricky.c:1893
#1  0x0000000000545b5c in docomplete (lst=4) at zle_tricky.c:664


> and consequently increases the offset by one, but then
>

I think this diagnosis is wrong -- it's not that the offset is increased,
it's that zlemetaline is shortened (by removal of the single quote).

The end result is the same, though -- the start of the word is calculated
by subtracting the offset from the current position, and the resulting
index is off the left end.

Having gotten that far, though, I don't know how to fix it.
>

The following may do it?  Completion tests still pass.  Without the change:

% autoload compinit zed
% compinit -D
% zstyle \* format %d
% alias a='<TAB>
% alias a=

With this change the vanishing quote mark no longer vanishes and a
description appears:

% autoload compinit zed
% compinit -D
% zstyle \* format %d
% alias a='<TAB>
`alias definition', `regular alias', `global alias', or `suffix alias'
alias definition
% alias a='

And the crash no longer happens when something appears after the single
quote.

I note that offs gets changed in the loop in an outer "else"-branch when
foredel/backdel are not called, too.  However, I'm not certain that the
edit should appear in BOTH hunks below.  Can anyone find any other test
cases that pass through this code?

diff --git a/Src/Zle/zle_tricky.c b/Src/Zle/zle_tricky.c
index fdd1687..2c24a13 100644
--- a/Src/Zle/zle_tricky.c
+++ b/Src/Zle/zle_tricky.c
@@ -1897,6 +1897,7 @@ get_comp_string(void)
                            zlemetacs = wb;
                    }
                    we -= skipchars;
+                   offs -= skipchars;
                }
            } else {
                ocs = zlemetacs;
@@ -1910,6 +1911,7 @@ get_comp_string(void)
                if (wb > zlemetacs)
                    zlemetacs = wb;
                we -= skipchars;
+               offs -= skipchars;
            }
            /* we need to get rid of all the quotation bits... */
            while (skipchars--)

Re: [BUG] Zsh crashes when using autocomplete because of memory unsafety (double free)

[Bart Schaefer]

(Apologies that the previous post was not in plain text mode, gmail
reacts to the type of the message being replied-to ...)

On Sun, Jun 28, 2020 at 12:27 PM Bart Schaefer
<schaefer@brasslantern.com> wrote:
>
> With this change the vanishing quote mark no longer vanishes and a description appears:
>

This might actually fix some other obscure bugs/oddities in completion
where things mysteriously disappear from the command line.

Here's the patch again in case HTML mangled it for the previous message:

diff --git a/Src/Zle/zle_tricky.c b/Src/Zle/zle_tricky.c
index fdd1687..2c24a13 100644
--- a/Src/Zle/zle_tricky.c
+++ b/Src/Zle/zle_tricky.c
@@ -1897,6 +1897,7 @@ get_comp_string(void)
                            zlemetacs = wb;
                    }
                    we -= skipchars;
+                   offs -= skipchars;
                }
            } else {
                ocs = zlemetacs;
@@ -1910,6 +1911,7 @@ get_comp_string(void)
                if (wb > zlemetacs)
                    zlemetacs = wb;
                we -= skipchars;
+               offs -= skipchars;
            }
            /* we need to get rid of all the quotation bits... */
            while (skipchars--)

Re: [BUG] Zsh crashes when using autocomplete because of memory unsafety (double free)

[Bart Schaefer]

On Sun, Jun 28, 2020 at 12:35 PM Bart Schaefer
<schaefer@brasslantern.com> wrote:
>
> Here's the patch again in case HTML mangled it for the previous message:

Sigh, I didn't think it could be this simple.  Now instead of
segfaulting with the cursor between the single quote and the double
quote, it segfaults if the cursor is after the double quote:

% alias a='"<TAB>
16: compcore.c:1678: expecting 'x' at offset -1 of "x""

Program received signal SIGSEGV, Segmentation fault.

Also with completeinword NOT set, the single quote still vanishes in this case:

% alias a='<TAB>
% alias a=

So the patch is sort of on the right track, but still not the whole
story.  Turning this back over to others, I probably won't have a
chance to look at it again for a few days.

Re: [BUG] Zsh crashes when using autocomplete because of memory unsafety (double free)

[Bart Schaefer]

On Sun, Jun 28, 2020 at 8:48 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> So the patch is sort of on the right track, but still not the whole
> story.  Turning this back over to others, I probably won't have a
> chance to look at it again for a few days.

Spent a little while on this today.

This seems to be related to get_comp_string() and the way completion
really wants to remove quotation marks.  E.g.:

% alias a='echo z'<TAB>
% alias a=echo\ zsh

When doing complete-in-word after an open-quote AND there are other
characters on the line that have to be backslash-escaped in order to
remove the quotes, the process of updating the line becomes confused.
For example, starting from:

% alias a="echo foo bar

Complete-in-word after "echo" crashes, and after "foo" produces the
"expect x at offset" debugging message.

It does seem to require the collusion of "compset -q" to actually
crash it.  The recalculation of where words begin and end leads to
mis-indexing.

Again this is as far as I've gotten.  If anyone else wants to have a
go, by all means.

Re: [BUG] Zsh crashes when using autocomplete because of memory unsafety (double free)

[Petr Šťastný]

[-- Attachment #1.1: Type: text/plain, Size: 589 bytes --]

Daniel Shahaf wrote on Fri, 26 Jun 2020 16:24:05 +0000:
> Petr, could you please check whether the issue reproduces if you
«unsetopt completeinword»?

I can confirm the issue is gone afterwards, nothing happens at all after
trying to autocomplete. I don't have any experience debugging this kind
of thing, so if there is anything I can do, please let me know.

Also, I'm sorry for late reply. I just discovered my selfhosted mail was
down for like a week so I didn't receive any mails. Yey. But I should
reply faster now.

-- 
Petr Šťastný
FIT ČVUT

soptik.tech



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]