Re: [PATCH] Improve DST handling (Bug 23102, Bug 21942, Bug 18018, Bug, 23259, CVE-2011-0536 ).

[Carlos O'Donell <carlos at redhat dot com>]

On 06/06/2018 01:28 PM, Florian Weimer wrote:
> On 06/06/2018 07:10 PM, Carlos O'Donell wrote:
>> On 06/06/2018 12:30 PM, Andreas Schwab wrote:
>>> On Jun 06 2018, Carlos O'Donell <carlos@redhat.com> wrote:
>>>
>>>> +  /* Find longest valid input sequence.  */
>>>> +  ilen = 0;
>>>> +  while ((input[ilen] >= 'A' && input[ilen] <= 'Z')
>>>> +     || (input[ilen] >= 'a' && input[ilen] <= 'z')
>>>> +     || (input[ilen] >= '0' && input[ilen] <= '9')
>>>> +     || (input[ilen] == '_'))
>>>> +    ++ilen;
>>>> +
>>>> +  rlen = strlen (ref);
>>>> +
>>>> +  /* Can't be the DST we are looking for.  */
>>>> +  if (rlen != ilen)
>>>> +    return 0;
>>>
>>> Why do you need that?  Just compare, then check the next character.
>>
>> Are you suggesting that:
>> ~~~
>> rlen = strlen (ref);
>>
>> /* Can't be the DST we are looking for.  */
>> if (rlen != ilen)
>>    return 0;
>> ~~~
>> Can be dropped because we are going to compare the strings anyway?
>>
>> I can do that.
> 
> If you compare the lengths first, yo ucan use memcmp.
> 
> You could compute the length of ref in an inline wrapper function, so that GCC will turn it into a compile-time constant.

The memcmp is a good suggestion.

Like so?

 
-  len = 0;
-  while (name[len] == str[len] && name[len] != '\0')
-    ++len;
+  /* Find longest valid input sequence.  */
+  ilen = 0;
+  while ((input[ilen] >= 'A' && input[ilen] <= 'Z')
+        || (input[ilen] >= 'a' && input[ilen] <= 'z')
+        || (input[ilen] >= '0' && input[ilen] <= '9')
+        || (input[ilen] == '_'))
+    ++ilen;
+
+  rlen = strlen (ref);
+
+  /* Can't be the DST we are looking for.  */
+  if (rlen != ilen)
+    return 0;
+
+  /* Compare the DST (no strncmp this early in startup).  */
+  if (memcmp (input, ref, ilen) != 0)
+    return 0;


The inline wrapper seems overkill for the rare is_dst() case
continuing past the strchr for '$'.

Cheers,
Carlos.