Bug 12393 (CVE-2011-1658) - ld.so: insecure handling of privileged programs' RPATHs with $ORIGIN (CVE-2011-1658)
Summary: ld.so: insecure handling of privileged programs' RPATHs with $ORIGIN (CVE-201...
Status: RESOLVED FIXED
Alias: CVE-2011-1658
Product: glibc
Classification: Unclassified
Component: libc (show other bugs)
Version: 2.12
: P2 normal
Target Milestone: ---
Assignee: Ulrich Drepper
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-12 14:47 UTC by Tomas Hoger
Modified: 2014-06-27 12:31 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security+


Attachments
proposed patch (598 bytes, patch)
2011-04-22 01:41 UTC, Petr Baudis
Details | Diff
updated patch (1.79 KB, patch)
2011-05-05 16:52 UTC, Petr Baudis
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tomas Hoger 2011-01-12 14:47:39 UTC
ld.so currently expands $ORIGIN in privileged programs' RPATH when $ORIGIN is listed alone (see _dl_dst_count and is_dst):

http://sourceware.org/git/?p=glibc.git;a=blob;f=elf/dl-load.c;h=41b5ce76;hb=master#l220

A local user can make ld.so load malicious DSO if she has write access to some directory on the same file system as:

$ ln /path/to/suid
$ LD_PRELOAD=payload ./suid


$ORIGIN is not expanded if it's not the only thing in RPATH, e.g. in cases like $ORIGIN/../lib, as DL_DST_COUNT() returns 0 and expand_dynamic_string_token() uses strdup rather than _dl_dst_substitute():

http://sourceware.org/git/?p=glibc.git;a=blob;f=elf/dl-load.c;h=41b5ce76;hb=master#l322

If some privileged program is built with such RPATH, malicious DSO can be loaded regardless of the file system boundaries as:

$ mkdir '$ORIGIN' lib
$ ln -s payload lib/lib-required-by-privileged-program.so
$ /path/to/suid

ld.so searches relative to the CWD.


Few possible fixes were proposed recently, such as:

http://sourceware.org/git/?p=glibc.git;a=commitdiff;h=96611391
http://sourceware.org/ml/libc-hacker/2010-12/msg00001.html

The problem with that approach is that with l_origin == -1, _dl_dst_substitute() expands "$ORIGIN" to "", which again triggers search staring from the CWD and can be abused as e.g.:

$ LD_PRELOAD=payload /path/to/suid


First two issues affect multiple glibc versions back, the third one was tested with Fedora glibc 2.12.2-1 packages.
Comment 1 Tomas Hoger 2011-04-11 14:22:23 UTC
All mentioned cases now seem to be addressed in Andreas' fedora master git branch.  Following seem to be the relevant commits:

http://sourceware.org/git/?p=glibc.git;a=commitdiff;h=96611391
http://sourceware.org/git/?p=glibc.git;a=commitdiff;h=101fdc24
http://sourceware.org/git/?p=glibc.git;a=commitdiff;h=049b59f7

(In reply to comment #0)
> ld.so currently expands $ORIGIN in privileged programs' RPATH when $ORIGIN is
> listed alone (see _dl_dst_count and is_dst)

is_dst() was changed by the above patches to flag $ORIGIN as recognized DST even when in __libc_enable_secure mode.  $ORIGIN is no longer expanded for executables, but it is for libraries.

> $ORIGIN is not expanded if it's not the only thing in RPATH, e.g. in cases
> like $ORIGIN/../lib, as DL_DST_COUNT() returns 0 and
> expand_dynamic_string_token() uses strdup rather than _dl_dst_substitute()

As is_dst() no longer ignores $ORIGIN for privileged programs, DL_DST_COUNT() does not return 0 any more and _dl_dst_substitute() is called.


Few comments regarding the applied changes:

- following comment in _dl_dst_substitute() is not completely correct:
    /* $ORIGIN is not expanded for SUID/GUID programs
       (except if it is $ORIGIN alone) and it must always
       appear first in path.  */
Expansion does not happen for SUID/SGID programs, but does for the libs they use.

- following comment is bit misleading:
    /* Also skip following colon if this is the first rpath
       element, but keep an empty element at the end.  */
Colon is skipped if there was no output written to the result buffer yet, so it may happen multiple times if multiple rpath elements are skipped (e.g. $ORIGIN:$ORIGIN:/lib).

- is_dst() no longer uses start argument passed to it.  _dl_dst_count() only uses start to pass it to is_dst().
Comment 2 Petr Baudis 2011-04-16 00:41:46 UTC
The initial comment is highly confusing, since when it says "ld.so currently expands $ORIGIN..." it talks NOT about master, but just about the fedora/ branches! In master, $ORIGIN is still expanded even for setuid programs, making just the hardlink attack possible in case someone is foolish enough to compile their setuid program with rpath $ORIGIN.

Therefore, the bug pretty much just describes which changes need to be covered when restricting $ORIGIN usage in setuid programs in master. An additional Ulrich's requirement is to whitelist system directories as $ORIGIN values even in setuid programs.
Comment 3 Tomas Hoger 2011-04-17 20:48:57 UTC
(In reply to comment #2)
> The initial comment is highly confusing, since when it says "ld.so currently
> expands $ORIGIN..." it talks NOT about master, but just about the fedora/
> branches!

Petr, I don't understand what confused you so highly.  Initial comment does point out several problematic cases, but it makes it clear that the first two affect currently used glibc versions (and various past versions, it seems the behaviour has not changed for 7+ years), and the third one was an issue that a *proposed* patch to address previous issues introduces, as that problem was not mentioned in the relevant libc-hacker thread.

> In master, $ORIGIN is still expanded even for setuid programs

As mentioned in comment #0, with an exception mentioned there too.  I fail to see how this contradicts the part of the comment #0 you quoted.
Comment 4 Petr Baudis 2011-04-18 14:14:35 UTC
You are right. I got a bit lost in the maze of all the RH bugs and advisories and in the end misinterpreted your comment. I was confused, not the bug. :-)
Comment 5 Tomas Hoger 2011-04-18 16:13:38 UTC
No worries, thank for looking!
Comment 6 Petr Baudis 2011-04-22 01:41:55 UTC
Created attachment 5684 [details]
proposed patch

Ok, here is a patch I'd propose instead of the Fedora patches (96611391 and the two followups). It is a rather simple approach of reusing the existing logic for checking against insecure paths in rpath. Am I missing something, is there a case this mishandles?
Comment 7 Tomas Hoger 2011-04-26 13:27:51 UTC
(In reply to comment #6)
> Am I missing something, is there a case this mishandles?

Seems to work correctly in all cases I previous pointed out.  Tested with the patch applied on top of vanilla 2.13.
Comment 8 Andreas Schwab 2011-05-03 11:14:05 UTC
This will reject any non-system library directory, which means a suid executable will no longer be able to use private libraries, even if they would be secure.
Comment 9 Petr Baudis 2011-05-05 16:52:15 UTC
Created attachment 5708 [details]
updated patch

Fair point. The updated patch is to replace the original one, instead relaxing the original restriction on $ORIGIN not being alone in the path. Instead, in case of a setuid binary, it verifies fully expanded and /../-normalized path elements where $ORIGIN has occurred against the list of trusted paths.
Comment 10 Ulrich Drepper 2011-05-07 15:46:09 UTC
I mostly agree with the last patch.  It contained some bugs and removed a check which has to stay.  I've checked in an updated patch.
Comment 11 Andreas Schwab 2011-05-09 08:40:44 UTC
This comment:

	      /* In SUID/SGID programs, after $ORIGIN expansion the
		 normalized path must be rooted in one of the trusted
		 directories.  */

is bogus.  In a privileged program $ORIGIN is not expanded except when isolated, and the binary will never be in a directory that is considered trusted (only library directories are).  Also, check_for_trusted is only reset when the test for trusted directories has succeeded, if it didn't the next path element will be checked as well.  It also does not address the problem of $ORIGIN/../lib not being expanded but accepted.
Comment 12 Petr Baudis 2011-05-09 10:24:19 UTC
Good catch with the elem_trusted variable not being reset!

What prevents $ORIGIN that is not alone in the path from being expanded? The point of this change is to allow that to happen securely.
Comment 13 Ulrich Drepper 2011-05-11 04:17:27 UTC
There were a few issues which should be fixed now.  Subdirs of trusted directories are allowed.
Comment 14 Jackie Rosen 2014-02-16 19:41:13 UTC Comment hidden (spam)