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.
This commit is contained in:
Florian Weimer 2016-04-29 10:35:34 +02:00
parent 137fe72eca
commit 4ab2ab03d4
3 changed files with 83 additions and 62 deletions

View File

@ -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-04-29 Florian Weimer <fweimer@redhat.com>
glob: Simplify and document the interface for the GLOB_ALTDIRFUNC

5
NEWS
View File

@ -27,7 +27,10 @@ Version 2.24
Security related changes:
[Add security related changes here]
* 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)
The following bugs are resolved with this release:

View File

@ -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
/* 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