[PATCH] elf: never leave $ORIGIN unexpanded

["Dmitry V. Levin" <ldv at altlinux dot org>]

Before this change, if a privileged executable's rpath element contained
$ORIGIN in a position that is not allowed for substitution in privileged
executables, this rpath element remained unexpanded, leading to the
following behaviour:

$ rm -rf '$ORIGIN' && mkdir -m0700 '$ORIGIN' &&
  ln -snf /dev/null '$ORIGIN/libc.so.6' &&
  echo 'int main(){}' |gcc -xc - -Wl,-rpath,'./$ORIGIN' &&
  chgrp -h another_group a.out && chmod 02710 a.out && ./a.out
./a.out: error while loading shared libraries: ./$ORIGIN/libc.so.6: file too short

After this change, such rpath elements will be discarded, making $ORIGIN
substitution rules closer to substitution rules of other dynamic string
tokens.

---
 ChangeLog     |  8 ++++++++
 elf/dl-load.c | 32 +++++++++++++++++---------------
 2 files changed, 25 insertions(+), 15 deletions(-)

This change started its life as commit
glibc-2.13-1002-g207e77fd3f0a94acdf0557608dd4f10ce0e0f22f
and has been rebased and reviewed many times.

It's in Fedora since glibc-2.13.90-12 (13.05.2011).

2017-12-27  Andreas Schwab  <schwab@redhat.com>
	    Dmitry V. Levin  <ldv@altlinux.org>

	* elf/dl-load.c (is_dst): Remove parameter secure, all callers
	changed.  Move check for valid use of $ORIGIN ...
	(_dl_dst_substitute): ... here.  Reset check_for_trusted when
	a path element is skipped.
 
diff --git a/elf/dl-load.c b/elf/dl-load.c
index ae86f0b..1263b54 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -180,7 +180,7 @@ is_trusted_path_normalize (const char *path, size_t len)
 
 
 static size_t
-is_dst (const char *start, const char *name, const char *str, int secure)
+is_dst (const char *start, const char *name, const char *str)
 {
   size_t len;
   bool is_curly = false;
@@ -208,11 +208,6 @@ is_dst (const char *start, const char *name, const char *str, int secure)
   else if (name[len] != '\0' && name[len] != '/')
     return 0;
 
-  if (__glibc_unlikely (secure)
-      && ((name[len] != '\0' && name[len] != '/')
-	  || (name != start + 1)))
-    return 0;
-
   return len;
 }
 
@@ -227,12 +222,10 @@ _dl_dst_count (const char *name)
     {
       size_t len;
 
-      /* $ORIGIN is not expanded for SUID/GUID programs (except if it
-	 is $ORIGIN alone) and it must always appear first in path.  */
       ++name;
-      if ((len = is_dst (start, name, "ORIGIN", __libc_enable_secure)) != 0
-	  || (len = is_dst (start, name, "PLATFORM", 0)) != 0
-	  || (len = is_dst (start, name, "LIB", 0)) != 0)
+      if ((len = is_dst (start, name, "ORIGIN")) != 0
+	  || (len = is_dst (start, name, "PLATFORM")) != 0
+	  || (len = is_dst (start, name, "LIB")) != 0)
 	++cnt;
 
       name = strchr (name + len, '$');
@@ -264,15 +257,23 @@ _dl_dst_substitute (struct link_map *l, const char *name, char *result)
 	  size_t len;
 
 	  ++name;
-	  if ((len = is_dst (start, name, "ORIGIN", __libc_enable_secure)) != 0)
+	  if ((len = is_dst (start, name, "ORIGIN")) != 0)
 	    {
-	      repl = l->l_origin;
+	      /* For SUID/GUID programs $ORIGIN must always appear
+		 first in a path element.  */
+	      if (__glibc_unlikely (__libc_enable_secure)
+		  && ((name[len] != '\0' && name[len] != '/')
+		      || (name != start + 1)))
+		repl = (const char *) -1;
+	      else
+		repl = l->l_origin;
+
 	      check_for_trusted = (__libc_enable_secure
 				   && l->l_type == lt_executable);
 	    }
-	  else if ((len = is_dst (start, name, "PLATFORM", 0)) != 0)
+	  else if ((len = is_dst (start, name, "PLATFORM")) != 0)
 	    repl = GLRO(dl_platform);
-	  else if ((len = is_dst (start, name, "LIB", 0)) != 0)
+	  else if ((len = is_dst (start, name, "LIB")) != 0)
 	    repl = DL_DST_LIB;
 
 	  if (repl != NULL && repl != (const char *) -1)
@@ -285,6 +286,7 @@ _dl_dst_substitute (struct link_map *l, const char *name, char *result)
 	      /* We cannot use this path element, the value of the
 		 replacement is unknown.  */
 	      wp = last_elem;
+	      check_for_trusted = false;
 	      break;
 	    }
 	  else
-- 
ldv