This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v2] Fix strstr bug with huge needles (bug 23637)



On 19/09/2018 08:31, Szabolcs Nagy wrote:
> On 17/09/18 11:37, Wilco Dijkstra wrote:
>> As reported in  https://sourceware.org/ml/libc-help/2018-09/msg00000.html ,
>> the generic strstr in GLIBC 2.28 fails to match huge needles.  The optimized
>> AVAILABLE macro reads ahead a large fixed amount to reduce the overhead of
>> repeatedly checking for the end of the string.  However if the needle length
>> is larger than this, two_way_long_needle may confuse this as meaning the end
>> of the string and return NULL.  This is fixed by adding the needle length to
>> the amount to read ahead.
> 
> i think this should be fixed and backported asap as it can
> easily cause user visible misbehaviour.
> 
>> diff --git a/string/strstr.c b/string/strstr.c
>> index 33acdc5442d3e9031298a56e4f52a19e2ffa7d84..f74d7189ed1319f6225525cc2d32380745de1523 100644
>> --- a/string/strstr.c
>> +++ b/string/strstr.c
>> @@ -33,8 +33,9 @@
>>     #define RETURN_TYPE char *
>>   #define AVAILABLE(h, h_l, j, n_l)                       \
>> -  (((j) + (n_l) <= (h_l)) || ((h_l) += __strnlen ((void*)((h) + (h_l)), 512), \
>> -                             (j) + (n_l) <= (h_l)))
>> +  (((j) + (n_l) <= (h_l)) \
>> +   || ((h_l) += __strnlen ((void*)((h) + (h_l)), (n_l) + 512), \
>> +       (j) + (n_l) <= (h_l)))
> 
> so the only diff is the +n_l in the strnlen limit.
> (same in strcasestr).
> 
> i think we can safely assume that n_l + 512 does not
> overflow. (user object cannot be so close to SIZE_MAX,
> but may be glibc should document its assumptions about
> the maximum object size somewhere).
> 
> the fix looks ok to me.

Good catch, I would prefer to use a saturated math here instead and not
make such assumptions (even though it would incur in a small performance
hit for a usercase unlikely to happen).


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]