]>
Commit | Line | Data |
---|---|---|
b9215da1 MT |
1 | From 750a45a783906a19591fb8ff6b7841470f1f5701 Mon Sep 17 00:00:00 2001 |
2 | From: Siddhesh Poyarekar <siddhesh@sourceware.org> | |
3 | Date: Tue, 19 Sep 2023 18:39:32 -0400 | |
a61a21ef | 4 | Subject: [PATCH 27/44] tunables: Terminate if end of input is reached |
b9215da1 MT |
5 | (CVE-2023-4911) |
6 | ||
7 | The string parsing routine may end up writing beyond bounds of tunestr | |
8 | if the input tunable string is malformed, of the form name=name=val. | |
9 | This gets processed twice, first as name=name=val and next as name=val, | |
10 | resulting in tunestr being name=name=val:name=val, thus overflowing | |
11 | tunestr. | |
12 | ||
13 | Terminate the parsing loop at the first instance itself so that tunestr | |
14 | does not overflow. | |
15 | ||
16 | This also fixes up tst-env-setuid-tunables to actually handle failures | |
17 | correct and add new tests to validate the fix for this CVE. | |
18 | ||
19 | Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org> | |
20 | Reviewed-by: Carlos O'Donell <carlos@redhat.com> | |
21 | (cherry picked from commit 1056e5b4c3f2d90ed2b4a55f96add28da2f4c8fa) | |
22 | --- | |
23 | NEWS | 5 +++++ | |
24 | elf/dl-tunables.c | 17 +++++++++------- | |
25 | elf/tst-env-setuid-tunables.c | 37 +++++++++++++++++++++++++++-------- | |
26 | 3 files changed, 44 insertions(+), 15 deletions(-) | |
27 | ||
28 | diff --git a/NEWS b/NEWS | |
29 | index f1b1b0a3b4..bfcd46efa9 100644 | |
30 | --- a/NEWS | |
31 | +++ b/NEWS | |
32 | @@ -24,6 +24,11 @@ Security related changes: | |
33 | an application calls getaddrinfo for AF_INET6 with AI_CANONNAME, | |
34 | AI_ALL and AI_V4MAPPED flags set. | |
35 | ||
36 | + CVE-2023-4911: If a tunable of the form NAME=NAME=VAL is passed in the | |
37 | + environment of a setuid program and NAME is valid, it may result in a | |
38 | + buffer overflow, which could be exploited to achieve escalated | |
39 | + privileges. This flaw was introduced in glibc 2.34. | |
40 | + | |
41 | The following bugs are resolved with this release: | |
42 | ||
43 | [30723] posix_memalign repeatedly scans long bin lists | |
44 | diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c | |
45 | index 62b7332d95..cae67efa0a 100644 | |
46 | --- a/elf/dl-tunables.c | |
47 | +++ b/elf/dl-tunables.c | |
48 | @@ -180,11 +180,7 @@ parse_tunables (char *tunestr, char *valstring) | |
49 | /* If we reach the end of the string before getting a valid name-value | |
50 | pair, bail out. */ | |
51 | if (p[len] == '\0') | |
52 | - { | |
53 | - if (__libc_enable_secure) | |
54 | - tunestr[off] = '\0'; | |
55 | - return; | |
56 | - } | |
57 | + break; | |
58 | ||
59 | /* We did not find a valid name-value pair before encountering the | |
60 | colon. */ | |
61 | @@ -244,9 +240,16 @@ parse_tunables (char *tunestr, char *valstring) | |
62 | } | |
63 | } | |
64 | ||
65 | - if (p[len] != '\0') | |
66 | - p += len + 1; | |
67 | + /* We reached the end while processing the tunable string. */ | |
68 | + if (p[len] == '\0') | |
69 | + break; | |
70 | + | |
71 | + p += len + 1; | |
72 | } | |
73 | + | |
74 | + /* Terminate tunestr before we leave. */ | |
75 | + if (__libc_enable_secure) | |
76 | + tunestr[off] = '\0'; | |
77 | } | |
78 | ||
79 | /* Enable the glibc.malloc.check tunable in SETUID/SETGID programs only when | |
80 | diff --git a/elf/tst-env-setuid-tunables.c b/elf/tst-env-setuid-tunables.c | |
81 | index 7dfb0e073a..f0b92c97e7 100644 | |
82 | --- a/elf/tst-env-setuid-tunables.c | |
83 | +++ b/elf/tst-env-setuid-tunables.c | |
84 | @@ -50,6 +50,8 @@ const char *teststrings[] = | |
85 | "glibc.malloc.perturb=0x800:not_valid.malloc.check=2:glibc.malloc.mmap_threshold=4096", | |
86 | "glibc.not_valid.check=2:glibc.malloc.mmap_threshold=4096", | |
87 | "not_valid.malloc.check=2:glibc.malloc.mmap_threshold=4096", | |
88 | + "glibc.malloc.mmap_threshold=glibc.malloc.mmap_threshold=4096", | |
89 | + "glibc.malloc.check=2", | |
90 | "glibc.malloc.garbage=2:glibc.maoc.mmap_threshold=4096:glibc.malloc.check=2", | |
91 | "glibc.malloc.check=4:glibc.malloc.garbage=2:glibc.maoc.mmap_threshold=4096", | |
92 | ":glibc.malloc.garbage=2:glibc.malloc.check=1", | |
93 | @@ -68,6 +70,8 @@ const char *resultstrings[] = | |
94 | "glibc.malloc.perturb=0x800:glibc.malloc.mmap_threshold=4096", | |
95 | "glibc.malloc.mmap_threshold=4096", | |
96 | "glibc.malloc.mmap_threshold=4096", | |
97 | + "glibc.malloc.mmap_threshold=glibc.malloc.mmap_threshold=4096", | |
98 | + "", | |
99 | "", | |
100 | "", | |
101 | "", | |
102 | @@ -81,11 +85,18 @@ test_child (int off) | |
103 | { | |
104 | const char *val = getenv ("GLIBC_TUNABLES"); | |
105 | ||
106 | + printf (" [%d] GLIBC_TUNABLES is %s\n", off, val); | |
107 | + fflush (stdout); | |
108 | if (val != NULL && strcmp (val, resultstrings[off]) == 0) | |
109 | return 0; | |
110 | ||
111 | if (val != NULL) | |
112 | - printf ("[%d] Unexpected GLIBC_TUNABLES VALUE %s\n", off, val); | |
113 | + printf (" [%d] Unexpected GLIBC_TUNABLES VALUE %s, expected %s\n", | |
114 | + off, val, resultstrings[off]); | |
115 | + else | |
116 | + printf (" [%d] GLIBC_TUNABLES environment variable absent\n", off); | |
117 | + | |
118 | + fflush (stdout); | |
119 | ||
120 | return 1; | |
121 | } | |
122 | @@ -106,21 +117,26 @@ do_test (int argc, char **argv) | |
123 | if (ret != 0) | |
124 | exit (1); | |
125 | ||
126 | - exit (EXIT_SUCCESS); | |
127 | + /* Special return code to make sure that the child executed all the way | |
128 | + through. */ | |
129 | + exit (42); | |
130 | } | |
131 | else | |
132 | { | |
133 | - int ret = 0; | |
134 | - | |
135 | /* Spawn tests. */ | |
136 | for (int i = 0; i < array_length (teststrings); i++) | |
137 | { | |
138 | char buf[INT_BUFSIZE_BOUND (int)]; | |
139 | ||
140 | - printf ("Spawned test for %s (%d)\n", teststrings[i], i); | |
141 | + printf ("[%d] Spawned test for %s\n", i, teststrings[i]); | |
142 | snprintf (buf, sizeof (buf), "%d\n", i); | |
143 | + fflush (stdout); | |
144 | if (setenv ("GLIBC_TUNABLES", teststrings[i], 1) != 0) | |
145 | - exit (1); | |
146 | + { | |
147 | + printf (" [%d] Failed to set GLIBC_TUNABLES: %m", i); | |
148 | + support_record_failure (); | |
149 | + continue; | |
150 | + } | |
151 | ||
152 | int status = support_capture_subprogram_self_sgid (buf); | |
153 | ||
154 | @@ -128,9 +144,14 @@ do_test (int argc, char **argv) | |
155 | if (WEXITSTATUS (status) == EXIT_UNSUPPORTED) | |
156 | return EXIT_UNSUPPORTED; | |
157 | ||
158 | - ret |= status; | |
159 | + if (WEXITSTATUS (status) != 42) | |
160 | + { | |
161 | + printf (" [%d] child failed with status %d\n", i, | |
162 | + WEXITSTATUS (status)); | |
163 | + support_record_failure (); | |
164 | + } | |
165 | } | |
166 | - return ret; | |
167 | + return 0; | |
168 | } | |
169 | } | |
170 | ||
171 | -- | |
172 | 2.39.2 | |
173 |