ld.so: Reject overly long LD_AUDIT path elements

Also only process the last LD_AUDIT entry.

diff --git a/ChangeLog b/ChangeLog
index d3dfbe48b1ac9738bdd2dea95d900cb52cb893f5..5c4815eb431852ca437d5dfcb7a70b86c049eb7b 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2017-06-19  Florian Weimer  <fweimer@redhat.com>
+
+       * elf/rtld.c (audit_list_string): New variable.
+       (audit_list): Update comment.
+       (struct audit_list_iter): Define.
+       (audit_list_iter_init, audit_list_iter_next): New function.
+       (dl_main): Use struct audit_list_iter to process audit modules.
+       (process_dl_audit): Call dso_name_valid_for_suid.
+       (process_envvars): Set audit_list_string instead of calling
+       process_dl_audit.
+
 2017-06-19  Florian Weimer  <fweimer@redhat.com>
 
        * elf/rtld.c (SECURE_NAME_LIMIT, SECURE_PATH_LIMIT): Define.

diff --git a/elf/rtld.c b/elf/rtld.c
index 86ae20c83fd7e34d04e9529a82071e9782ef5a95..65647fb1c8167ac7432c212975e4027f2c46b560 100644 (file)
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -129,13 +129,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[SECURE_NAME_LIMIT];
+};
+
+/* 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 < sizeof (iter->fname))
+           {
+             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;
+}
+
 #ifndef HAVE_INLINED_SYSCALLS
 /* Set nonzero during loading and initialization of executable and
    libraries, cleared before the executable's entry point runs.  This
@@ -1305,11 +1383,13 @@ of this helper program; chances are you did not intend to run this program.\n\
     GL(dl_rtld_map).l_tls_modid = _dl_next_tls_modid ();
 
   /* If we have auditing DSOs to load, do it now.  */
-  if (__glibc_unlikely (audit_list != NULL))
+  bool need_security_init = true;
+  if (__glibc_unlikely (audit_list != NULL)
+      || __glibc_unlikely (audit_list_string != NULL))
     {
-      /* 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.  */
@@ -1320,9 +1400,14 @@ of this helper program; chances are you did not intend to run this program.\n\
         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
@@ -1331,7 +1416,7 @@ of this helper program; chances are you did not intend to run this program.\n\
             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;
@@ -1344,7 +1429,7 @@ of this helper program; chances are you did not intend to run this program.\n\
            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);
            }
@@ -1448,10 +1533,7 @@ ERROR: ld.so: object '%s' cannot be loaded as audit interface: %s; ignored.\n",
                  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.  */
@@ -1715,7 +1797,7 @@ ERROR: ld.so: object '%s' cannot be loaded as audit interface: %s; ignored.\n",
   if (tcbp == NULL)
     tcbp = init_tls ();
 
-  if (__glibc_likely (audit_list == NULL))
+  if (__glibc_likely (need_security_init))
     /* Initialize security features.  But only if we have not done it
        earlier.  */
     security_init ();
@@ -2346,9 +2428,7 @@ process_dl_audit (char *str)
   char *p;
 
   while ((p = (strsep) (&str, ":")) != NULL)
-    if (p[0] != '\0'
-       && (__builtin_expect (! __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.  */
@@ -2412,7 +2492,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: