Re: [PATCH] don't let char class disturb end finding

I was able to reproduce this,
 ==28848== Invalid write of size 1
==28848==    at 0x67D7E7F: parse_class (complete.c:551)
==28848==    by 0x67D7993: parse_pattern (complete.c:435)
==28848==    by 0x67D7585: parse_cmatcher (complete.c:317)
==28848==    by 0x67D8A86: bin_compadd (complete.c:833)
==28848==    by 0x410C5E: execbuiltin (builtin.c:506)
==28848==    by 0x43A65C: execcmd_exec (exec.c:4266)
==28848==    by 0x433C51: execpline2 (exec.c:2050)
==28848==    by 0x432894: execpline (exec.c:1775)
==28848==  Address 0x77bed65 is 0 bytes after a block of size 5 alloc'd
==28848==    at 0x48397B5: malloc (in
/usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==28848==    by 0x4739A1: zalloc (mem.c:966)
==28848==    by 0x67D7C5B: parse_class (complete.c:509)
==28848==    by 0x67D7993: parse_pattern (complete.c:435)
==28848==    by 0x67D7585: parse_cmatcher (complete.c:317)

==28848== Invalid write of size 1
==28848==    at 0x67D7EA9: parse_class (complete.c:557)
==28848==    by 0x67D7993: parse_pattern (complete.c:435)

==28848== Invalid read of size 1
==28848==    at 0x483F894: __strlen_sse2 (in
/usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==28848==    by 0x4AA2DA: ztrdup (string.c:68)
==28848==    by 0x67D714B: cp_cpattern_element (complete.c:199)
==28848==    by 0x67D71A3: cpcpattern (complete.c:223)
==28848==    by 0x67D702A: cpcmatcher (complete.c:165)
==28848==    by 0x67D8B30: bin_compadd (complete.c:847)


(I temporarily changed the zhalloc on complete.c:509 to a regular
zalloc to make things easier for valgrind).


reproducer:
% foo() { compadd -M 'M:[[:a:]123456]=[[:b:]abcdef]' foo bar baz };
compdef foo foo
% foo <tab>


On Thu, Jun 18, 2015 at 3:49 AM Han Pingtian <[email protected]> wrote:
>
> On Wed, Jun 17, 2015 at 08:23:05AM -0700, Bart Schaefer wrote:
> > On Jun 17,  2:16pm, Han Pingtian wrote:
> > }
> > } This patch try to fix this problem:
> > }
> > } compadd -M '[[:lower:]123456]=...' will cause the end of class to be the
> > } ']' before 1 and will alloc range of memory less than enough for the
> > } cpattern.
> >
> > I don't see anything obviously wrong with the patch, but when I try the
> > above example directly I get "unknown match specification character `['"
> > both before and after applying your patch (and no complaints of memory
> > misuse from valgrind, even before your patch).  Is that the correct
> > example to reproduce the error?
> >
> > I also tried '[[:lower:]123456]=[[:upper:]abcdef]' with the same result.
>
> Sorry, my fault. It should be 'M:[[:lower:]123456]=[[:upper:]abcdef]'
> and for triggering memory misuse, I think we should use someting like
> 'M:[[:a:]123456]=[[:b:]abcdef]'. Then
>
>  469     optr = p->u.str = zhalloc((optr-iptr) + 1);
>
> will alloc a memory 5 bytes long, but latter it will put 6 bytes into
> this memory.
>


-- 
Mikael Magnusson