Re: [PATCH v6] 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 09:23 AM, Andreas Schwab wrote:
> On Jun 12 2018, Andreas Schwab <schwab@suse.de> wrote:
> 
>> On Jun 12 2018, Carlos O'Donell <carlos@redhat.com> wrote:
>>
>>> +  /* 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] == '_')))
>>
>> That will always reject ${FOO}.  If is_curly is true, the other checks
>> must not be performed.
> 
> Sorry, this is of course rubbish.  This is ok.

No worries. Thanks for your time and review.

I had a private question about this sequence, which I thought was odd enough
that I added a test case for it.

Consider:

[/${unrecognized$LIB}]

In this case it's really 3 sequences.

[${unrecognized] - An invalid DST that is handled as a literal.

[$LIB] - A valid DST which is substituted as [lib64].

[}] - A literal curly.

I added a test for this and it passes as expected, but obviously the
directory name is a bit odd.

Test 1i: Test that RPATH of /${unrecognized$LIB} loads from /${unrecognizedlib64} as expected.
PASS: Correct primary library.

This result is a consequence of the interpretation that curly-braced names
must still follow the valid-character rules, and that the longest name
ends at the next non-name character (the '$' from '$LIB' in this case) and
thus the parser considers it an invalid curly-braced name and ignores it.

I do not think it valuable to allow any characters inside the curly-braces
(treating it like an escape sequence). I believe the ELF gABI is clear enough
in this case:
http://www.sco.com/developers/gabi/latest/ch5.dynamic.html#shobj_dependencies
~~~
Within a string provided by dynamic array entries with the DT_NEEDED or 
DT_RUNPATH tags and in pathnames passed as parameters to the dlopen() 
routine, a dollar sign ($) introduces a substitution sequence. This 
sequence consists of the dollar sign immediately followed by either 
the longest name sequence or a name contained within left and right 
                               ^^^^
braces ({) and (}). A name is a sequence of bytes that start with 
                      ^^^^
either a letter or an underscore followed by zero or more letters, 
digits or underscores. If a dollar sign is not immediately followed 
by a name or a brace-enclosed name, the behavior of the dynamic 
linker is unspecified. 
~~~

I use '^^^' to highlight that 'name' is used consistently to refer
to a DST name sequence of valid characters.

So what we have implemented is sensible to me.

Cheers,
Carlos.