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/12/2018 03:31 AM, Andreas Schwab wrote:
> On Jun 11 2018, Carlos O'Donell <carlos@redhat.com> wrote:
> 
>> By validating it against the abstract definition of the ELF gABI spec 
>> for a DST name it prevents any future caller from accidentally straying
>> outside of that contract without having to make two changes: (1) change 
>> the comparison DST and (2) change what we accept in is_dst().
> 
> Why would we add any use of is_dst with an invalid DST?  That doesn't
> make sense.

An invalid DST would be a mistake. The point is to catch mistakes by
enforcing the rules for valid DSTs.

>> Also I find the code ends up being simpler.
> 
> I don't agree.  It is overly complex for no reason.

There is a reason. You don't think the reason has any value though,
and that's different. That's also OK.

I'll post a v6 with the validation removed, I appreciate your point
of view. I'll work with Florian to get strncmp usage in place.

Would you be OK with something like this?

  /* Is a ${...} input sequence?  */
  if (input[0] == '{')
    {
      is_curly = true;
      ++input;
    }

  /* Check for matching name, following closing curly brace (if
     required), or trailing characters which are part of an
     identifier.  */
  size_t rlen = strlen (ref);
  if (strncmp (input, ref, rlen) != 0
      || (is_curly && input[rlen] != '}')
      || ((input[rlen] >= 'A' && input[rlen] <= 'Z')
          || (input[rlen] >= 'a' && input[rlen] <= 'z')
          || (input[rlen] >= '0' && input[rlen] <= '9')
          || (input[rlen] == '_')))
    return 0;
  
  if (is_curly)
    /* Count the two curly braces.  */
    return ilen + 2;
  else
    return ilen;

Cheers,
Carlos.g