current/SOURCES/134-CVE-2016-3706-getaddrinfo-stack-overflow-in-hostent-.patch

From 762aafec34478bcef01a16acf1959732ab8bb2b6 Mon Sep 17 00:00:00 2001
From: Florian Weimer <fweimer@redhat.com>
Date: Fri, 29 Apr 2016 10:35:34 +0200
Subject: [PATCH] CVE-2016-3706: getaddrinfo: stack overflow in hostent conversion [BZ #20010]

When converting a struct hostent response to struct gaih_addrtuple, the
gethosts macro (which is called from gaih_inet) used alloca, without
malloc fallback for large responses.  This commit changes this code to
use calloc unconditionally.

This commit also consolidated a second hostent-to-gaih_addrtuple
conversion loop (in gaih_inet) to use the new conversion function.

(cherry picked from commit 4ab2ab03d4351914ee53248dc5aef4a8c88ff8b9)
---
 ChangeLog                   |   10 +++
 NEWS                        |    7 ++-
 sysdeps/posix/getaddrinfo.c |  130 +++++++++++++++++++++++--------------------
 3 files changed, 85 insertions(+), 62 deletions(-)

#diff --git a/ChangeLog b/ChangeLog
#index c55bff5..671f2bf 100644
#--- a/ChangeLog
#+++ b/ChangeLog
#@@ -1,3 +1,13 @@
#+2016-04-29  Florian Weimer  <fweimer@redhat.com>
#+
#+      [BZ #20010]
#+      CVE-2016-3706
#+      * sysdeps/posix/getaddrinfo.c
#+      (convert_hostent_to_gaih_addrtuple): New function.
#+      (gethosts): Call convert_hostent_to_gaih_addrtuple.
#+      (gaih_inet): Use convert_hostent_to_gaih_addrtuple to convert
#+      AF_INET data.
#+
# 2016-05-04  Florian Weimer  <fweimer@redhat.com>
# 
#       [BZ #19779]
#diff --git a/NEWS b/NEWS
#index 00a8add..5304a05 100644
#--- a/NEWS
#+++ b/NEWS
#@@ -12,7 +12,7 @@ Version 2.19.1
#   15946, 16545, 16574, 16623, 16657, 16695, 16743, 16758, 16759, 16760,
#   16878, 16882, 16885, 16916, 16932, 16943, 16958, 17048, 17062, 17069,
#   17079, 17137, 17153, 17213, 17263, 17269, 17325, 17555, 17905, 18007,
#-  18032, 18080, 18240, 18287, 18508, 18905, 19779, 19879.
#+  18032, 18080, 18240, 18287, 18508, 18905, 19779, 19879, 20010.
# 
# * A buffer overflow in gethostbyname_r and related functions performing DNS
#   requests has been fixed.  If the NSS functions were called with a
#@@ -72,6 +72,11 @@ Version 2.19.1
# * The glob function suffered from a stack-based buffer overflow when it was
#   called with the GLOB_ALTDIRFUNC flag and encountered a long file name.
#   Reported by Alexander Cherepanov.  (CVE-2016-1234)
#+
#+* Previously, getaddrinfo copied large amounts of address data to the stack,
#+  even after the fix for CVE-2013-4458 has been applied, potentially
#+  resulting in a stack overflow.  getaddrinfo now uses a heap allocation
#+  instead.  Reported by Michael Petlan.  (CVE-2016-3706)
# 
# Version 2.19
# 
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index d2283bc..df6ce8b 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -168,9 +168,58 @@ gaih_inet_serv (const char *servicename, const struct gaih_typeproto *tp,
   return 0;
 }
 
+/* Convert struct hostent to a list of struct gaih_addrtuple objects.
+   h_name is not copied, and the struct hostent object must not be
+   deallocated prematurely.  *RESULT must be NULL or a pointer to an
+   object allocated using malloc, which is freed.  */
+static bool
+convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
+                                  int family,
+                                  struct hostent *h,
+                                  struct gaih_addrtuple **result)
+{
+  free (*result);
+  *result = NULL;
+
+  /* Count the number of addresses in h->h_addr_list.  */
+  size_t count = 0;
+  for (char **p = h->h_addr_list; *p != NULL; ++p)
+    ++count;
+
+  /* Report no data if no addresses are available, or if the incoming
+     address size is larger than what we can store.  */
+  if (count == 0 || h->h_length > sizeof (((struct gaih_addrtuple) {}).addr))
+    return true;
+
+  struct gaih_addrtuple *array = calloc (count, sizeof (*array));
+  if (array == NULL)
+    return false;
+
+  for (size_t i = 0; i < count; ++i)
+    {
+      if (family == AF_INET && req->ai_family == AF_INET6)
+       {
+         /* Perform address mapping. */
+         array[i].family = AF_INET6;
+         memcpy(array[i].addr + 3, h->h_addr_list[i], sizeof (uint32_t));
+         array[i].addr[2] = htonl (0xffff);
+       }
+      else
+       {
+         array[i].family = family;
+         memcpy (array[i].addr, h->h_addr_list[i], h->h_length);
+       }
+      array[i].next = array + i + 1;
+    }
+  array[0].name = h->h_name;
+  array[count - 1].next = NULL;
+
+  *result = array;
+  return true;
+}
+
 #define gethosts(_family, _type) \
  {                                                                           \
-  int i;                                                                     \
   int herrno;                                                                \
   struct hostent th;                                                         \
   struct hostent *h;                                                         \
@@ -219,36 +268,23 @@ gaih_inet_serv (const char *servicename, const struct gaih_typeproto *tp,
     }                                                                        \
   else if (h != NULL)                                                        \
     {                                                                        \
-      for (i = 0; h->h_addr_list[i]; i++)                                    \
+      /* Make sure that addrmem can be freed.  */                            \
+      if (!malloc_addrmem)                                                   \
+       addrmem = NULL;                                                       \
+      if (!convert_hostent_to_gaih_addrtuple (req, _family,h, &addrmem))      \
        {                                                                     \
-         if (*pat == NULL)                                                   \
-           {                                                                 \
-             *pat = __alloca (sizeof (struct gaih_addrtuple));               \
-             (*pat)->scopeid = 0;                                            \
-           }                                                                 \
-         uint32_t *addr = (*pat)->addr;                                      \
-         (*pat)->next = NULL;                                                \
-         (*pat)->name = i == 0 ? strdupa (h->h_name) : NULL;                 \
-         if (_family == AF_INET && req->ai_family == AF_INET6)               \
-           {                                                                 \
-             (*pat)->family = AF_INET6;                                      \
-             addr[3] = *(uint32_t *) h->h_addr_list[i];                      \
-             addr[2] = htonl (0xffff);                                       \
-             addr[1] = 0;                                                    \
-             addr[0] = 0;                                                    \
-           }                                                                 \
-         else                                                                \
-           {                                                                 \
-             (*pat)->family = _family;                                       \
-             memcpy (addr, h->h_addr_list[i], sizeof(_type));                \
-           }                                                                 \
-         pat = &((*pat)->next);                                              \
+         _res.options |= old_res_options & RES_USE_INET6;                    \
+         result = -EAI_SYSTEM;                                               \
+         goto free_and_return;                                               \
        }                                                                     \
+      *pat = addrmem;                                                        \
+      /* The conversion uses malloc unconditionally.  */                     \
+      malloc_addrmem = true;                                                 \
                                                                              \
       if (localcanon !=        NULL && canon == NULL)                                \
        canon = strdupa (localcanon);                                         \
                                                                              \
-      if (_family == AF_INET6 && i > 0)                                              \
+      if (_family == AF_INET6 && *pat != NULL)                               \
        got_ipv6 = true;                                                      \
     }                                                                        \
  }
@@ -612,44 +648,16 @@ gaih_inet (const char *name, const struct gaih_service *service,
                {
                  if (h != NULL)
                    {
-                     int i;
-                     /* We found data, count the number of addresses.  */
-                     for (i = 0; h->h_addr_list[i]; ++i)
-                       ;
-                     if (i > 0 && *pat != NULL)
-                       --i;
-
-                     if (__libc_use_alloca (alloca_used
-                                            + i * sizeof (struct gaih_addrtuple)))
-                       addrmem = alloca_account (i * sizeof (struct gaih_addrtuple),
-                                                 alloca_used);
-                     else
-                       {
-                         addrmem = malloc (i
-                                           * sizeof (struct gaih_addrtuple));
-                         if (addrmem == NULL)
-                           {
-                             result = -EAI_MEMORY;
-                             goto free_and_return;
-                           }
-                         malloc_addrmem = true;
-                       }
-
-                     /* Now convert it into the list.  */
-                     struct gaih_addrtuple *addrfree = addrmem;
-                     for (i = 0; h->h_addr_list[i]; ++i)
+                     /* We found data, convert it.  */
+                     if (!convert_hostent_to_gaih_addrtuple
+                         (req, AF_INET, h, &addrmem))
                        {
-                         if (*pat == NULL)
-                           {
-                             *pat = addrfree++;
-                             (*pat)->scopeid = 0;
-                           }
-                         (*pat)->next = NULL;
-                         (*pat)->family = AF_INET;
-                         memcpy ((*pat)->addr, h->h_addr_list[i],
-                                 h->h_length);
-                         pat = &((*pat)->next);
+                         result = -EAI_MEMORY;
+                         goto free_and_return;
                        }
+                     *pat = addrmem;
+                     /* The conversion uses malloc unconditionally.  */
+                     malloc_addrmem = true;
                    }
                }
              else
-- 
1.7.1

1	From 762aafec34478bcef01a16acf1959732ab8bb2b6 Mon Sep 17 00:00:00 2001
2	From: Florian Weimer <fweimer@redhat.com>
3	Date: Fri, 29 Apr 2016 10:35:34 +0200
4	Subject: [PATCH] CVE-2016-3706: getaddrinfo: stack overflow in hostent conversion [BZ #20010]
5
6	When converting a struct hostent response to struct gaih_addrtuple, the
7	gethosts macro (which is called from gaih_inet) used alloca, without
8	malloc fallback for large responses. This commit changes this code to
9	use calloc unconditionally.
10
11	This commit also consolidated a second hostent-to-gaih_addrtuple
12	conversion loop (in gaih_inet) to use the new conversion function.
13
14	(cherry picked from commit 4ab2ab03d4351914ee53248dc5aef4a8c88ff8b9)
15	---
16	ChangeLog \| 10 +++
17	NEWS \| 7 ++-
18	sysdeps/posix/getaddrinfo.c \| 130 +++++++++++++++++++++++--------------------
19	3 files changed, 85 insertions(+), 62 deletions(-)
20
21	#diff --git a/ChangeLog b/ChangeLog
22	#index c55bff5..671f2bf 100644
23	#--- a/ChangeLog
24	#+++ b/ChangeLog
25	#@@ -1,3 +1,13 @@
26	#+2016-04-29 Florian Weimer <fweimer@redhat.com>
27	#+
28	#+ [BZ #20010]
29	#+ CVE-2016-3706
30	#+ * sysdeps/posix/getaddrinfo.c
31	#+ (convert_hostent_to_gaih_addrtuple): New function.
32	#+ (gethosts): Call convert_hostent_to_gaih_addrtuple.
33	#+ (gaih_inet): Use convert_hostent_to_gaih_addrtuple to convert
34	#+ AF_INET data.
35	#+
36	# 2016-05-04 Florian Weimer <fweimer@redhat.com>
37	#
38	# [BZ #19779]
39	#diff --git a/NEWS b/NEWS
40	#index 00a8add..5304a05 100644
41	#--- a/NEWS
42	#+++ b/NEWS
43	#@@ -12,7 +12,7 @@ Version 2.19.1
44	# 15946, 16545, 16574, 16623, 16657, 16695, 16743, 16758, 16759, 16760,
45	# 16878, 16882, 16885, 16916, 16932, 16943, 16958, 17048, 17062, 17069,
46	# 17079, 17137, 17153, 17213, 17263, 17269, 17325, 17555, 17905, 18007,
47	#- 18032, 18080, 18240, 18287, 18508, 18905, 19779, 19879.
48	#+ 18032, 18080, 18240, 18287, 18508, 18905, 19779, 19879, 20010.
49	#
50	# * A buffer overflow in gethostbyname_r and related functions performing DNS
51	# requests has been fixed. If the NSS functions were called with a
52	#@@ -72,6 +72,11 @@ Version 2.19.1
53	# * The glob function suffered from a stack-based buffer overflow when it was
54	# called with the GLOB_ALTDIRFUNC flag and encountered a long file name.
55	# Reported by Alexander Cherepanov. (CVE-2016-1234)
56	#+
57	#+* Previously, getaddrinfo copied large amounts of address data to the stack,
58	#+ even after the fix for CVE-2013-4458 has been applied, potentially
59	#+ resulting in a stack overflow. getaddrinfo now uses a heap allocation
60	#+ instead. Reported by Michael Petlan. (CVE-2016-3706)
61	#
62	# Version 2.19
63	#
64	diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
65	index d2283bc..df6ce8b 100644
66	--- a/sysdeps/posix/getaddrinfo.c
67	+++ b/sysdeps/posix/getaddrinfo.c
68	@@ -168,9 +168,58 @@ gaih_inet_serv (const char servicename, const struct gaih_typeproto tp,
69	return 0;
70	}
71
72	+/* Convert struct hostent to a list of struct gaih_addrtuple objects.
73	+ h_name is not copied, and the struct hostent object must not be
74	+ deallocated prematurely. *RESULT must be NULL or a pointer to an
75	+ object allocated using malloc, which is freed. */
76	+static bool
77	+convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
78	+ int family,
79	+ struct hostent *h,
80	+ struct gaih_addrtuple **result)
81	+{
82	+ free (*result);
83	+ *result = NULL;
84	+
85	+ /* Count the number of addresses in h->h_addr_list. */
86	+ size_t count = 0;
87	+ for (char *p = h->h_addr_list; p != NULL; ++p)
88	+ ++count;
89	+
90	+ /* Report no data if no addresses are available, or if the incoming
91	+ address size is larger than what we can store. */
92	+ if (count == 0 \|\| h->h_length > sizeof (((struct gaih_addrtuple) {}).addr))
93	+ return true;
94	+
95	+ struct gaih_addrtuple array = calloc (count, sizeof (array));
96	+ if (array == NULL)
97	+ return false;
98	+
99	+ for (size_t i = 0; i < count; ++i)
100	+ {
101	+ if (family == AF_INET && req->ai_family == AF_INET6)
102	+ {
103	+ /* Perform address mapping. */
104	+ array[i].family = AF_INET6;
105	+ memcpy(array[i].addr + 3, h->h_addr_list[i], sizeof (uint32_t));
106	+ array[i].addr[2] = htonl (0xffff);
107	+ }
108	+ else
109	+ {
110	+ array[i].family = family;
111	+ memcpy (array[i].addr, h->h_addr_list[i], h->h_length);
112	+ }
113	+ array[i].next = array + i + 1;
114	+ }
115	+ array[0].name = h->h_name;
116	+ array[count - 1].next = NULL;
117	+
118	+ *result = array;
119	+ return true;
120	+}
121	+
122	#define gethosts(_family, _type) \
123	{ \
124	- int i; \
125	int herrno; \
126	struct hostent th; \
127	struct hostent *h; \
128	@@ -219,36 +268,23 @@ gaih_inet_serv (const char servicename, const struct gaih_typeproto tp,
129	} \
130	else if (h != NULL) \
131	{ \
132	- for (i = 0; h->h_addr_list[i]; i++) \
133	+ /* Make sure that addrmem can be freed. */ \
134	+ if (!malloc_addrmem) \
135	+ addrmem = NULL; \
136	+ if (!convert_hostent_to_gaih_addrtuple (req, _family,h, &addrmem)) \
137	{ \
138	- if (*pat == NULL) \
139	- { \
140	- *pat = __alloca (sizeof (struct gaih_addrtuple)); \
141	- (*pat)->scopeid = 0; \
142	- } \
143	- uint32_t addr = (pat)->addr; \
144	- (*pat)->next = NULL; \
145	- (*pat)->name = i == 0 ? strdupa (h->h_name) : NULL; \
146	- if (_family == AF_INET && req->ai_family == AF_INET6) \
147	- { \
148	- (*pat)->family = AF_INET6; \
149	- addr[3] = (uint32_t ) h->h_addr_list[i]; \
150	- addr[2] = htonl (0xffff); \
151	- addr[1] = 0; \
152	- addr[0] = 0; \
153	- } \
154	- else \
155	- { \
156	- (*pat)->family = _family; \
157	- memcpy (addr, h->h_addr_list[i], sizeof(_type)); \
158	- } \
159	- pat = &((*pat)->next); \
160	+ _res.options \|= old_res_options & RES_USE_INET6; \
161	+ result = -EAI_SYSTEM; \
162	+ goto free_and_return; \
163	} \
164	+ *pat = addrmem; \
165	+ /* The conversion uses malloc unconditionally. */ \
166	+ malloc_addrmem = true; \
167	\
168	if (localcanon != NULL && canon == NULL) \
169	canon = strdupa (localcanon); \
170	\
171	- if (_family == AF_INET6 && i > 0) \
172	+ if (_family == AF_INET6 && *pat != NULL) \
173	got_ipv6 = true; \
174	} \
175	}
176	@@ -612,44 +648,16 @@ gaih_inet (const char name, const struct gaih_service service,
177	{
178	if (h != NULL)
179	{
180	- int i;
181	- /* We found data, count the number of addresses. */
182	- for (i = 0; h->h_addr_list[i]; ++i)
183	- ;
184	- if (i > 0 && *pat != NULL)
185	- --i;
186	-
187	- if (__libc_use_alloca (alloca_used
188	- + i * sizeof (struct gaih_addrtuple)))
189	- addrmem = alloca_account (i * sizeof (struct gaih_addrtuple),
190	- alloca_used);
191	- else
192	- {
193	- addrmem = malloc (i
194	- * sizeof (struct gaih_addrtuple));
195	- if (addrmem == NULL)
196	- {
197	- result = -EAI_MEMORY;
198	- goto free_and_return;
199	- }
200	- malloc_addrmem = true;
201	- }
202	-
203	- /* Now convert it into the list. */
204	- struct gaih_addrtuple *addrfree = addrmem;
205	- for (i = 0; h->h_addr_list[i]; ++i)
206	+ /* We found data, convert it. */
207	+ if (!convert_hostent_to_gaih_addrtuple
208	+ (req, AF_INET, h, &addrmem))
209	{
210	- if (*pat == NULL)
211	- {
212	- *pat = addrfree++;
213	- (*pat)->scopeid = 0;
214	- }
215	- (*pat)->next = NULL;
216	- (*pat)->family = AF_INET;
217	- memcpy ((*pat)->addr, h->h_addr_list[i],
218	- h->h_length);
219	- pat = &((*pat)->next);
220	+ result = -EAI_MEMORY;
221	+ goto free_and_return;
222	}
223	+ *pat = addrmem;
224	+ /* The conversion uses malloc unconditionally. */
225	+ malloc_addrmem = true;
226	}
227	}
228	else
229	--
230	1.7.1
231