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

[Florian Weimer]

On 06/08/2018 08:03 AM, Carlos O'Donell wrote:
> On 06/08/2018 01:51 AM, Florian Weimer wrote:
>> On 06/08/2018 07:45 AM, Carlos O'Donell wrote:
>>> +Â Â Â Â Â Â Â Â Â  /* For SUID/GUID programs we normally ignore the path with
>>> +         a DST in DT_RUNPATH, or DT_RPATH.  However, there is
>>> +Â Â Â Â Â Â Â Â  one exception to this rule, and it is:
>>> +
>>> +Â Â Â Â Â Â Â Â Â Â  * $ORIGIN appears first in the path element, and is
>>> +Â Â Â Â Â Â Â Â Â Â Â Â  the only thing in the element or is immediately
>>> +Â Â Â Â Â Â Â Â Â Â Â Â  followed by a path separator and the rest of the
>>> +Â Â Â Â Â Â Â Â Â Â Â Â  path.
>>> +
>>> +Â Â Â Â Â Â Â Â Â Â  * The path element is rooted in a trusted directory.
>>> +
>>> +Â Â Â Â Â Â Â Â  This exception allows such programs to reference
>>> +Â Â Â Â Â Â Â Â  shared libraries in subdirectories of trusted
>>> +         directories.  The use case is one of general
>>> +Â Â Â Â Â Â Â Â  organization and deployment flexibility.
>>> +Â Â Â Â Â Â Â Â  Trusted directories are usually such paths as "/lib64"
>>> +         or "/lib".  */
>>> +Â Â Â Â Â Â Â Â Â  if (__glibc_unlikely (__libc_enable_secure)
>>> +Â Â Â Â Â Â Â Â Â  && !((input == start + 1
>>> +Â Â Â Â Â Â Â Â Â Â Â  || (input > start + 1 && input[-2] == '\0'))
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â  && (input[len] == '\0' || input[len] == '/')))
>>> +Â Â Â Â Â Â Â  repl = (const char *) -1;
>>
>> The comment does not match the code: The code checks that $ORIGIN
>> comes first in the *path*, not *path element* (hence the need for the
>> start variable).  I'm not sure what the right behavior is here.
>> Going by path element seems more correct.
> Sorry, there is a bit of confusion regarding terminology here.
> 
> When you have multiple colon separated paths in DT_RUNPATH or
> DT_RPATH, what is each path called? Are they path elements?
> 
> Or are we calling path elements only those things that are
> between slashes in a singular path?
> 
> It seems context dependent, but I understand the confusion.
> 
> [/path/dir:/path/dir1:/path/dir2]
>   ^^^^^^^^^ --- What do we call this in the context of multiple paths?
> 
> Just a 'path' ?
> 
> I consider the following valid:
> 
> [$ORIGIN/../$LIB]

I'm actually asking about this:

[$LIB:$ORIGIN/../$LIB]

Doesn't the current code reject this?

(Some of the callers split at :, not /.)

Thanks,
Florian