Possible NULL deref in cfp_matcher_range?

Possible NULL deref in cfp_matcher_range?

[Mikael Magnusson]

Checking some stuff with clang, and it complained here;

static char *
cfp_matcher_range(Cmatcher *ms, char *add)
{
    Cmatcher *mp, m;
    int len = 0, mt;
    char *ret = NULL, *p = NULL, *adds = add;

    /*
     * Do this twice:  once to work out the length of the
     * string in len, the second time to build it in ret.
     * This is probably worthwhile because otherwise memory
     * management is difficult.
     */
    for (;;) {
    MB_METACHARINIT();
    for (mp = ms; *add; ) {
        convchar_t addc;
        int addlen;

        addlen = MB_METACHARLENCONV(add, &addc);
#ifdef MULTIBYTE_SUPPORT
        if (addc == WEOF)
        addc = (wchar_t)(*p == Meta ? p[1] ^ 32 : *p);
#endif


First run through this code p is NULL, and the other places all (at
least the ones I looked at) protect accesses to p by "if (ret)" (which
is set at the same time as p). Should this do that, and/or do
something clever with len at the same time? Is there any way to
actually trigger addc to be WEOF here? Presumably if it is WEOF on the
second run when  is set, then it was also WEOF the first time.

-- 
Mikael Magnusson

Re: Possible NULL deref in cfp_matcher_range?

[Peter Stephenson]

On Sun, 5 Jul 2015 16:52:19 +0200
Mikael Magnusson <mikachu@gmail.com> wrote:
> Checking some stuff with clang, and it complained here;
>
>         addc = (wchar_t)(*p == Meta ? p[1] ^ 32 : *p);
> 
> First run through this code p is NULL, and the other places all (at
> least the ones I looked at) protect accesses to p by "if (ret)" (which
> is set at the same time as p). Should this do that, and/or do
> something clever with len at the same time? Is there any way to
> actually trigger addc to be WEOF here? Presumably if it is WEOF on the
> second run when  is set, then it was also WEOF the first time.

Actually, I suspect it's supposed to do this with the input chaaracter,
not the output...

pws

diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c
index 27938c1..e5db086 100644
--- a/Src/Zle/computil.c
+++ b/Src/Zle/computil.c
@@ -4196,7 +4196,7 @@ cfp_matcher_range(Cmatcher *ms, char *add)
 	    addlen = MB_METACHARLENCONV(add, &addc);
 #ifdef MULTIBYTE_SUPPORT
 	    if (addc == WEOF)
-		addc = (wchar_t)(*p == Meta ? p[1] ^ 32 : *p);
+		addc = (wchar_t)(*add == Meta ? add[1] ^ 32 : *add);
 #endif
 
 	    if (!(m = *mp)) {

Re: Possible NULL deref in cfp_matcher_range?

[Mikael Magnusson]

On Sun, Jul 5, 2015 at 7:34 PM, Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
> On Sun, 5 Jul 2015 16:52:19 +0200
> Mikael Magnusson <mikachu@gmail.com> wrote:
>> Checking some stuff with clang, and it complained here;
>>
>>         addc = (wchar_t)(*p == Meta ? p[1] ^ 32 : *p);
>>
>> First run through this code p is NULL, and the other places all (at
>> least the ones I looked at) protect accesses to p by "if (ret)" (which
>> is set at the same time as p). Should this do that, and/or do
>> something clever with len at the same time? Is there any way to
>> actually trigger addc to be WEOF here? Presumably if it is WEOF on the
>> second run when  is set, then it was also WEOF the first time.
>
> Actually, I suspect it's supposed to do this with the input chaaracter,
> not the output...

That makes even more sense. :)

-- 
Mikael Magnusson