From c69d4a0f680a24fdbe323764a50382ad324041e9 Mon Sep 17 00:00:00 2001 From: Florian Weimer Date: Mon, 19 Jun 2017 22:32:12 +0200 Subject: [PATCH] ld.so: Reject overly long LD_AUDIT path elements Also only process the last LD_AUDIT entry. (cherry picked from commit 81b82fb966ffbd94353f793ad17116c6088dedd9) diff --git a/elf/rtld.c b/elf/rtld.c index 4badcf6..369724b 100644 --- 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 @@ -1303,11 +1381,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. */ @@ -1318,9 +1398,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 @@ -1329,7 +1414,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; @@ -1342,7 +1427,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); } @@ -1446,10 +1531,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. */ @@ -1713,7 +1795,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 (); @@ -2344,9 +2426,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. */ @@ -2410,7 +2490,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: -- 2.9.3