diff options
author | Florian Weimer <fweimer@redhat.com> | 2016-04-29 10:35:34 +0200 |
---|---|---|
committer | Gabriel F. T. Gomes <gftg@linux.vnet.ibm.com> | 2016-05-24 11:35:31 -0300 |
commit | 1029487ab3b2ef65ec9364cb15c44ee9c571224f (patch) | |
tree | bcdf22236e9d5a9a13688ff53cc0d0700f366e7c | |
parent | edbab2450a084a275d418fba871a8c7b48001a71 (diff) | |
download | glibc-1029487ab3b2ef65ec9364cb15c44ee9c571224f.zip glibc-1029487ab3b2ef65ec9364cb15c44ee9c571224f.tar.gz glibc-1029487ab3b2ef65ec9364cb15c44ee9c571224f.tar.bz2 |
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)
-rw-r--r-- | ChangeLog | 10 | ||||
-rw-r--r-- | NEWS | 5 | ||||
-rw-r--r-- | sysdeps/posix/getaddrinfo.c | 130 |
3 files changed, 84 insertions, 61 deletions
@@ -1,3 +1,13 @@ +2016-05-24 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-04-28 Stefan Liebler <stli@linux.vnet.ibm.com> [BZ #18508] @@ -13,6 +13,11 @@ Version 2.20.1 17625, 17630, 17801, 18032, 18080, 18508, 18665, 18694, 18928, 19018, 19682. +* 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) + * A stack-based buffer overflow was found in libresolv when invoked from libnss_dns, allowing specially crafted DNS responses to seize control of execution flow in the DNS client. The buffer overflow occurs in diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c index 8f392b9..e63454a 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 |