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

[Carlos O'Donell]

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 do *not* consider the following valid:

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

Even though they are the same path.

I believe the glibc exception was a very *narrow* exception which
allowed only $ORIGIN-starting paths rooted in trusted directories
to be allowed for SUID/SGID. It's just easier to explain to
developers.

In the abstract all we really care about is that the result be
rooted in a trusted directory. So in that case the appearance
of *any* DST would enable check_for_trusted. That would be an
expansion of what is allowed for SUID/SGID though, and I'm not
sure I want to do that any further.

Thoughts?

Cheers,
Carlos.