Description: Prevent unqualified linker paths. A while ago, I suggested that I'd been working on glibc with regard to making it handle runtime linking more securely. Today, I finally managed to get a patch together which I think is worth sharing. I'd therefore like to issue a request for comments for the attached patch, and enquire if any distros feel like incorporating it? A few points on the rationale... . Why? . Over the last couple of years I've spent a good deal of time dealing with vendors who, for one reason or another have shipped binaries where it is possible to inject "untrusted" code into running processes, notably but not exclusively via DT_RPATH. . What's the fix? . More often than not, the underlying issue is an empty element within the DT_RPATH header or equivalent. Sometimes it's not, but even in those cases, it is largely that one or more elements isn't qualifed (i.e. it doesn't start with /). The attached patch fixes this, by ignoring any elements of DT_RPATH, LD_LIBRARY_PATH that do not start with a /, and/or junking any use of dlopen where the filename is likewise unqualified. . Won't this break stuff? . Maybe (certainly it is means a change to glibc behaviour), but more often than not, the fact that a given binary currently works in an unsafe way is a bug - and an exploitable one at that. Moreoever, Solaris has had a similar sanitity check (in their case only for privileged setuid binaries) for a good number of years without serious incident. I believe we should be fixing software that exhibits the behaviour I've described, but this patch will (I think) kill the bug class irrespective of that. . Further thoughts? . The patch attached is the most robust variant I've produced in that it kills unqualified linker paths, irrespective of the privilege or otherwise of the affected binary. We could kill the checks for non-setuid binaries or we could add some additional errors in such cases. I did experiment with only checking a subset of cases (namely where LD_LIBRARY_PATH itself is set) if the process wasn't privileged, but in the end, I concluded that the loading of any unqualified linker path could provide an exploit vector (if the non-setuid binary is executed from a privileged process etc) and so erred on the side of caution. A useful variant of my patch for auditors is one that logs dangerous conditions rather than reject them outright, but I'm unconviced that it is helpful for the masses. Author: Tim Brown Origin: other, https://www.nth-dimension.org.uk/downloads.php?id=100 Last-Update: 2015-02-19 --- glibc-2.19.orig/elf/dl-load.c +++ glibc-2.19/elf/dl-load.c @@ -2203,6 +2203,35 @@ open_path (const char *name, size_t name if (this_dir->status[cnt] == nonexisting) continue; + if (INTUSE(__libc_enable_secure)) + { + if (this_dir->status[cnt] == insecure) + continue; + else + { + if (*(this_dir->dirname) != '/') + { + this_dir->status[cnt] = insecure; + continue; + } + } + } + else + { + /* We could actually be a bit cleverer here. ATM we just + forbid all unqualified paths even if it's not setuid. */ + if (this_dir->status[cnt] == insecure) + continue; + else + { + if (*(this_dir->dirname) != '/') + { + this_dir->status[cnt] = insecure; + continue; + } + } + } + buflen = ((char *) __mempcpy (__mempcpy (edp, capstr[cnt].str, capstr[cnt].len), @@ -2241,7 +2270,8 @@ open_path (const char *name, size_t name } /* Remember whether we found any existing directory. */ - here_any |= this_dir->status[cnt] != nonexisting; + /* Or if it was insecure. */ + here_any |= ((this_dir->status[cnt] != nonexisting) && (this_dir->status[cnt] != insecure)); if (fd != -1 && __builtin_expect (secure, 0) && INTUSE(__libc_enable_secure)) @@ -2532,20 +2562,32 @@ _dl_map_object (struct link_map *loader, } else { - /* The path may contain dynamic string tokens. */ - realname = (loader - ? expand_dynamic_string_token (loader, name, 0) - : local_strdup (name)); - if (realname == NULL) - fd = -1; + if (INTUSE(__libc_enable_secure) && (*name != '/')) + fd = -1; else - { - fd = open_verify (realname, &fb, - loader ?: GL(dl_ns)[nsid]._ns_loaded, 0, - &found_other_class, true); - if (__builtin_expect (fd, 0) == -1) - free (realname); - } + { + /* We could actually be a bit cleverer here. ATM we just + forbid all unqualified paths even if it's not setuid. */ + if (*name != '/') + fd = -1; + else + { + /* The path may contain dynamic string tokens. */ + realname = (loader + ? expand_dynamic_string_token (loader, name, 0) + : local_strdup (name)); + if (realname == NULL) + fd = -1; + else + { + fd = open_verify (realname, &fb, + loader ?: GL(dl_ns)[nsid]._ns_loaded, 0, + &found_other_class, true); + if (__builtin_expect (fd, 0) == -1) + free (realname); + } + } + } } #ifdef SHARED --- glibc-2.19.orig/sysdeps/generic/ldsodefs.h +++ glibc-2.19/sysdeps/generic/ldsodefs.h @@ -149,7 +149,7 @@ struct r_found_version /* We want to cache information about the searches for shared objects. */ -enum r_dir_status { unknown, nonexisting, existing }; +enum r_dir_status { unknown, nonexisting, existing, insecure }; struct r_search_path_elem {