develooper Front page | perl.perl5.porters | Postings from September 2016

Re: [perl #129267] Possible string overrun with invalid len in gv.c

Thread Previous | Thread Next
From:
demerphq
Date:
September 13, 2016 18:59
Subject:
Re: [perl #129267] Possible string overrun with invalid len in gv.c
Message ID:
CANgJU+VYmAeeFT_XQXqnZj_2BFdW77R6hb04H=WHs6uMvKMffA@mail.gmail.com
On 13 September 2016 at 20:55, Todd Rinaldo via RT
<perlbug-followup@perl.org> wrote:
> On Tue Sep 13 11:51:18 2016, demerphq wrote:
>
>> Well, that fixes /one/ issue. But I think there are more. I was
>> putting together a patch like this:
>>
>> $ git diff gv.c
>> diff --git a/gv.c b/gv.c
>> index 1bc8bf2..23700a0 100644
>> --- a/gv.c
>> +++ b/gv.c
>> @@ -1009,6 +1009,7 @@ GV *
>>  Perl_gv_fetchmethod_pvn_flags(pTHX_ HV *stash, const char *name,
>> const STRLEN len, U32 flags)
>>  {
>>      const char *nend;
>> +    const char * const name_end= name + len;
> s/char * const/char */
>
>>      const char *nsplit = NULL;
>>      GV* gv;
>>      HV* ostash = stash;
>> @@ -1028,15 +1029,15 @@ Perl_gv_fetchmethod_pvn_flags(pTHX_ HV *stash,
>> const char *name, const STRLE
>>            the error reporting code.  */
>>      }
>>
>> -    for (nend = name; *nend || nend != (origname + len); nend++) {
>> +    for (nend = name; nend < name_end && *nend; nend++) {
>>         if (*nend == '\'') {
>>             nsplit = nend;
>>             name = nend + 1;
> I assume you changed <= to < to address this? It's unclear to me the effects of name being an empty string.

If its an empty string it should be len == 0.

Notice i reversed the clauses as well. We dereference AFTER we have
assured ourselves that we are looking at memory we own.

>
>
>>         }
>> -       else if (*nend == ':' && *(nend + 1) == ':') {
>> -           nsplit = nend++;
>> -           name = nend + 1;
>> -       }
>> +        else if (*nend == ':' && nend+1 < name_end && *(nend + 1) == ':') {
>> +            nsplit = nend++;
>> +            name = nend + 1;
>> +        }

And again here. We have to recheck things so that we are sure we can
even look at nend + 1.

nend is a terrible name. Should be name_ptr or something like that.

Yves


-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About