| From 4ebd0c4191c6073cc8a7c5fdcf1d182c4719bcbb Mon Sep 17 00:00:00 2001 |
| From: Aurelien Jarno <aurelien@aurel32.net> |
| Date: Sat, 30 Dec 2017 10:54:23 +0100 |
| Subject: [PATCH] elf: Check for empty tokens before dynamic string token |
| expansion [BZ #22625] |
| |
| The fillin_rpath function in elf/dl-load.c loops over each RPATH or |
| RUNPATH tokens and interprets empty tokens as the current directory |
| ("./"). In practice the check for empty token is done *after* the |
| dynamic string token expansion. The expansion process can return an |
| empty string for the $ORIGIN token if __libc_enable_secure is set |
| or if the path of the binary can not be determined (/proc not mounted). |
| |
| Fix that by moving the check for empty tokens before the dynamic string |
| token expansion. In addition, check for NULL pointer or empty strings |
| return by expand_dynamic_string_token. |
| |
| The above changes highlighted a bug in decompose_rpath, an empty array |
| is represented by the first element being NULL at the fillin_rpath |
| level, but by using a -1 pointer in decompose_rpath and other functions. |
| |
| Changelog: |
| [BZ #22625] |
| * elf/dl-load.c (fillin_rpath): Check for empty tokens before dynamic |
| string token expansion. Check for NULL pointer or empty string possibly |
| returned by expand_dynamic_string_token. |
| (decompose_rpath): Check for empty path after dynamic string |
| token expansion. |
| (cherry picked from commit 3e3c904daef69b8bf7d5cc07f793c9f07c3553ef) |
| |
| Upstream-Status: Backport |
| CVE: CVE-2017-16997 |
| Signed-off-by: Armin Kuster <akuster@mvista.com> |
| |
| --- |
| ChangeLog | 10 ++++++++++ |
| NEWS | 4 ++++ |
| elf/dl-load.c | 49 +++++++++++++++++++++++++++++++++---------------- |
| 3 files changed, 47 insertions(+), 16 deletions(-) |
| |
| Index: git/NEWS |
| =================================================================== |
| --- git.orig/NEWS |
| +++ git/NEWS |
| @@ -215,6 +215,10 @@ Security related changes: |
| GLOB_NOESCAPE, could write past the end of a buffer while |
| unescaping user names. Reported by Tim RΓΌhsen. |
| |
| + CVE-2017-16997: Incorrect handling of RPATH or RUNPATH containing $ORIGIN |
| + for AT_SECURE or SUID binaries could be used to load libraries from the |
| + current directory. |
| + |
| The following bugs are resolved with this release: |
| |
| [984] network: Respond to changed resolv.conf in gethostbyname |
| Index: git/elf/dl-load.c |
| =================================================================== |
| --- git.orig/elf/dl-load.c |
| +++ git/elf/dl-load.c |
| @@ -433,32 +433,41 @@ fillin_rpath (char *rpath, struct r_sear |
| { |
| char *cp; |
| size_t nelems = 0; |
| - char *to_free; |
| |
| while ((cp = __strsep (&rpath, sep)) != NULL) |
| { |
| struct r_search_path_elem *dirp; |
| + char *to_free = NULL; |
| + size_t len = 0; |
| |
| - to_free = cp = expand_dynamic_string_token (l, cp, 1); |
| + /* `strsep' can pass an empty string. */ |
| + if (*cp != '\0') |
| + { |
| + to_free = cp = expand_dynamic_string_token (l, cp, 1); |
| |
| - size_t len = strlen (cp); |
| + /* expand_dynamic_string_token can return NULL in case of empty |
| + path or memory allocation failure. */ |
| + if (cp == NULL) |
| + continue; |
| + |
| + /* Compute the length after dynamic string token expansion and |
| + ignore empty paths. */ |
| + len = strlen (cp); |
| + if (len == 0) |
| + { |
| + free (to_free); |
| + continue; |
| + } |
| |
| - /* `strsep' can pass an empty string. This has to be |
| - interpreted as `use the current directory'. */ |
| - if (len == 0) |
| - { |
| - static const char curwd[] = "./"; |
| - cp = (char *) curwd; |
| + /* Remove trailing slashes (except for "/"). */ |
| + while (len > 1 && cp[len - 1] == '/') |
| + --len; |
| + |
| + /* Now add one if there is none so far. */ |
| + if (len > 0 && cp[len - 1] != '/') |
| + cp[len++] = '/'; |
| } |
| |
| - /* Remove trailing slashes (except for "/"). */ |
| - while (len > 1 && cp[len - 1] == '/') |
| - --len; |
| - |
| - /* Now add one if there is none so far. */ |
| - if (len > 0 && cp[len - 1] != '/') |
| - cp[len++] = '/'; |
| - |
| /* Make sure we don't use untrusted directories if we run SUID. */ |
| if (__glibc_unlikely (check_trusted) && !is_trusted_path (cp, len)) |
| { |
| @@ -621,6 +630,14 @@ decompose_rpath (struct r_search_path_st |
| necessary. */ |
| free (copy); |
| |
| + /* There is no path after expansion. */ |
| + if (result[0] == NULL) |
| + { |
| + free (result); |
| + sps->dirs = (struct r_search_path_elem **) -1; |
| + return false; |
| + } |
| + |
| sps->dirs = result; |
| /* The caller will change this value if we haven't used a real malloc. */ |
| sps->malloced = 1; |
| Index: git/ChangeLog |
| =================================================================== |
| --- git.orig/ChangeLog |
| +++ git/ChangeLog |
| @@ -1,3 +1,12 @@ |
| +2017-12-30 Aurelien Jarno <aurelien@aurel32.net> |
| + Dmitry V. Levin <ldv@altlinux.org> |
| + |
| + [BZ #22625] |
| + * elf/dl-load.c (fillin_rpath): Check for empty tokens before dynamic |
| + string token expansion. Check for NULL pointer or empty string possibly |
| + returned by expand_dynamic_string_token. |
| + (decompose_rpath): Check for empty path after dynamic string |
| + token expansion. |
| |
| 2017-10-20 Paul Eggert <eggert@cs.ucla.edu> |
| |