1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
|
From 24ee2a5b63d15cf45c43ec598f11fe59878982a8 Mon Sep 17 00:00:00 2001
From: Andreas Wellving <andreas.wellving@enea.com>
Date: Mon, 22 Oct 2018 11:17:18 +0200
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)
CVE: CVE-2017-16997
Upstream-Status: Backport [https://sourceware.org/git/gitweb.cgi?p=glibc.git;a=patch;h=21c5d14bfb4e08bee86f94fd815535d3be2c3869]
Signed-off-by: Andreas Wellving <andreas.wellving@enea.com>
---
ChangeLog | 10 ++++++++++
NEWS | 4 ++++
elf/dl-load.c | 49 +++++++++++++++++++++++++++++++++----------------
3 files changed, 47 insertions(+), 16 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index a0c2f51..ad380fd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+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-04-13 Florian Weimer <fweimer@redhat.com>
[BZ #21361]
diff --git a/NEWS b/NEWS
index 29e795a..195c06d 100644
--- a/NEWS
+++ b/NEWS
@@ -214,6 +214,10 @@ Security related changes:
* The xdr_bytes and xdr_string routines free the internally allocated buffer
if deserialization of the buffer contents fails for any reason.
+ 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:
[4099] stdio: Overly agressive caching by stream i/o functions.
diff --git a/elf/dl-load.c b/elf/dl-load.c
index a5318f9..bdb4484 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -433,31 +433,40 @@ fillin_rpath (char *rpath, struct r_search_path_elem **result, const char *sep,
{
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;
- /* `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;
- }
+ /* Compute the length after dynamic string token expansion and
+ ignore empty paths. */
+ len = strlen (cp);
+ if (len == 0)
+ {
+ free (to_free);
+ continue;
+ }
- /* Remove trailing slashes (except for "/"). */
- while (len > 1 && cp[len - 1] == '/')
- --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++] = '/';
+ /* 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_struct *sps,
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;
|
