commit TBD

Author: Florian Weimer <fweimer@redhat.com>

Date:   Fri May 19 17:46:47 2017 +0200

    rtld: Reject overly long LD_AUDIT path elements

Also only process the last LD_AUDIT entry.

Index: b/elf/rtld.c

===================================================================

--- a/elf/rtld.c

+++ b/elf/rtld.c

@@ -116,13 +116,91 @@ dso_name_valid_for_suid (const char *p)

   return *p != '\0';

 }

-/* List of auditing DSOs.  */

+/* LD_AUDIT variable contents.  Must be processed before the

+   audit_list below.  */

+const char *audit_list_string;

+

+/* Cyclic list of auditing DSOs.  audit_list->next is the first

+   element.  */

 static struct audit_list

 {

   const char *name;

   struct audit_list *next;

 } *audit_list;

+/* Iterator for audit_list_string followed by audit_list.  */

+struct audit_list_iter

+{

+  /* Tail of audit_list_string still needing processing, or NULL.  */

+  const char *audit_list_tail;

+

+  /* The list element returned in the previous iteration.  NULL before

+     the first element.  */

+  struct audit_list *previous;

+

+  /* Scratch buffer for returning a name which is part of

+     audit_list_string.  */

+  char fname[PATH_MAX];

+};

+

+/* Initialize an audit list iterator.  */

+static void

+audit_list_iter_init (struct audit_list_iter *iter)

+{

+  iter->audit_list_tail = audit_list_string;

+  iter->previous = NULL;

+}

+

+/* Iterate through both audit_list_string and audit_list.  */

+static const char *

+audit_list_iter_next (struct audit_list_iter *iter)

+{

+  if (iter->audit_list_tail != NULL)

+    {

+      /* First iterate over audit_list_string.  */

+      while (*iter->audit_list_tail != '\0')

+	{

+	  /* Split audit list at colon.  */

+	  size_t len = strcspn (iter->audit_list_tail, ":");

+	  if (len > 0 && len < PATH_MAX)

+	    {

+	      memcpy (iter->fname, iter->audit_list_tail, len);

+	      iter->fname[len] = '\0';

+	    }

+	  else

+	    /* Do not return this name to the caller.  */

+	    iter->fname[0] = '\0';

+

+	  /* Skip over the substring and the following delimiter.  */

+	  iter->audit_list_tail += len;

+	  if (*iter->audit_list_tail == ':')

+	    ++iter->audit_list_tail;

+

+	  /* If the name is valid, return it.  */

+	  if (dso_name_valid_for_suid (iter->fname))

+	    return iter->fname;

+	  /* Otherwise, wrap around and try the next name.  */

+	}

+      /* Fall through to the procesing of audit_list.  */

+    }

+

+  if (iter->previous == NULL)

+    {

+      if (audit_list == NULL)

+	/* No pre-parsed audit list.  */

+	return NULL;

+      /* Start of audit list.  The first list element is at

+	 audit_list->next (cyclic list).  */

+      iter->previous = audit_list->next;

+      return iter->previous->name;

+    }

+  if (iter->previous == audit_list)

+    /* Cyclic list wrap-around.  */

+    return NULL;

+  iter->previous = iter->previous->next;

+  return iter->previous->name;

+}

+

 /* Set nonzero during loading and initialization of executable and

    libraries, cleared before the executable's entry point runs.  This

    must not be initialized to nonzero, because the unused dynamic

@@ -1441,11 +1519,13 @@ of this helper program; chances are you

     GL(dl_rtld_map).l_tls_modid = _dl_next_tls_modid ();

   /* If we have auditing DSOs to load, do it now.  */

-  if (__builtin_expect (audit_list != NULL, 0))

+  bool need_security_init = true;

+  if (__builtin_expect (audit_list != NULL, 0)

+      || __builtin_expect (audit_list_string != NULL, 0))

     {

-      /* Iterate over all entries in the list.  The order is important.  */

       struct audit_ifaces *last_audit = NULL;

-      struct audit_list *al = audit_list->next;

+      struct audit_list_iter al_iter;

+      audit_list_iter_init (&al_iter);

       /* Since we start using the auditing DSOs right away we need to

 	 initialize the data structures now.  */

@@ -1456,9 +1536,14 @@ of this helper program; chances are you

 	 use different values (especially the pointer guard) and will

 	 fail later on.  */

       security_init ();

+      need_security_init = false;

-      do

+      while (true)

 	{

+	  const char *name = audit_list_iter_next (&al_iter);

+	  if (name == NULL)

+	    break;

+

 	  int tls_idx = GL(dl_tls_max_dtv_idx);

 	  /* Now it is time to determine the layout of the static TLS

@@ -1467,7 +1552,7 @@ of this helper program; chances are you

 	     no DF_STATIC_TLS bit is set.  The reason is that we know

 	     glibc will use the static model.  */

 	  struct dlmopen_args dlmargs;

-	  dlmargs.fname = al->name;

+	  dlmargs.fname = name;

 	  dlmargs.map = NULL;

 	  const char *objname;

@@ -1480,7 +1565,7 @@ of this helper program; chances are you

 	    not_loaded:

 	      _dl_error_printf ("\

 ERROR: ld.so: object '%s' cannot be loaded as audit interface: %s; ignored.\n",

-				al->name, err_str);

+				name, err_str);

 	      if (malloced)

 		free ((char *) err_str);

 	    }

@@ -1584,10 +1669,7 @@ ERROR: ld.so: object '%s' cannot be load

 		  goto not_loaded;

 		}

 	    }

-

-	  al = al->next;

 	}

-      while (al != audit_list->next);

       /* If we have any auditing modules, announce that we already

 	 have two objects loaded.  */

@@ -1851,7 +1933,7 @@ ERROR: ld.so: object '%s' cannot be load

   if (tcbp == NULL)

     tcbp = init_tls ();

-  if (__builtin_expect (audit_list == NULL, 1))

+  if (need_security_init)

     /* Initialize security features.  But only if we have not done it

        earlier.  */

     security_init ();

@@ -2495,9 +2577,7 @@ process_dl_audit (char *str)

   char *p;

   while ((p = (strsep) (&str, ":")) != NULL)

-    if (p[0] != '\0'

-	&& (__builtin_expect (! INTUSE(__libc_enable_secure), 1)

-	    || strchr (p, '/') == NULL))

+    if (dso_name_valid_for_suid (p))

       {

 	/* This is using the local malloc, not the system malloc.  The

 	   memory can never be freed.  */

@@ -2561,7 +2641,7 @@ process_envvars (enum mode *modep)

 	      break;

 	    }

 	  if (memcmp (envline, "AUDIT", 5) == 0)

-	    process_dl_audit (&envline[6]);

+	    audit_list_string = &envline[6];

 	  break;

 	case 7: