From 4ab2ab03d4351914ee53248dc5aef4a8c88ff8b9 Mon Sep 17 00:00:00 2001 From: Florian Weimer 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. Upstream-Status: Backport CVE: CVE-2016-3706 Signed-off-by: Armin Kuster --- ChangeLog | 10 ++++ NEWS | 5 +- sysdeps/posix/getaddrinfo.c | 130 +++++++++++++++++++++++--------------------- 3 files changed, 83 insertions(+), 62 deletions(-) Index: git/sysdeps/posix/getaddrinfo.c =================================================================== --- git.orig/sysdeps/posix/getaddrinfo.c +++ git/sysdeps/posix/getaddrinfo.c @@ -168,9 +168,58 @@ gaih_inet_serv (const char *servicename, 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, } \ 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 struc { 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 + /* We found data, convert it. */ + if (!convert_hostent_to_gaih_addrtuple + (req, AF_INET, h, &addrmem)) { - 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) - { - 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 Index: git/NEWS =================================================================== --- git.orig/NEWS +++ git/NEWS @@ -2,6 +2,11 @@ GNU C Library NEWS -- history of user-vi Copyright (C) 1992-2016 Free Software Foundation, Inc. See the end for copying conditions. +* 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) + Please send GNU C library bug reports via using `glibc' in the "product" field. Index: git/ChangeLog =================================================================== --- git.orig/ChangeLog +++ git/ChangeLog @@ -1,3 +1,13 @@ +2016-04-29 Florian Weimer + + [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-02-18 Adhemerval Zanella * configure: Regenerated.