From 973fe93a5675c42798b2161c6f29c01b0e243994 Mon Sep 17 00:00:00 2001 From: Siddhesh Poyarekar Date: Fri, 15 Sep 2023 13:51:12 -0400 Subject: [PATCH] getaddrinfo: Fix use after free in getcanonname (CVE-2023-4806) When an NSS plugin only implements the _gethostbyname2_r and _getcanonname_r callbacks, getaddrinfo could use memory that was freed during tmpbuf resizing, through h_name in a previous query response. The backing store for res->at->name when doing a query with gethostbyname3_r or gethostbyname2_r is tmpbuf, which is reallocated in gethosts during the query. For AF_INET6 lookup with AI_ALL | AI_V4MAPPED, gethosts gets called twice, once for a v6 lookup and second for a v4 lookup. In this case, if the first call reallocates tmpbuf enough number of times, resulting in a malloc, th->h_name (that res->at->name refers to) ends up on a heap allocated storage in tmpbuf. Now if the second call to gethosts also causes the plugin callback to return NSS_STATUS_TRYAGAIN, tmpbuf will get freed, resulting in a UAF reference in res->at->name. This then gets dereferenced in the getcanonname_r plugin call, resulting in the use after free. Fix this by copying h_name over and freeing it at the end. This resolves BZ #30843, which is assigned CVE-2023-4806. Signed-off-by: Siddhesh Poyarekar --- sysdeps/posix/getaddrinfo.c | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c index c91b281e..3973c13a 100644 --- a/sysdeps/posix/getaddrinfo.c +++ b/sysdeps/posix/getaddrinfo.c @@ -107,6 +107,12 @@ struct gaih_servtuple int port; }; +struct gaih_result +{ + char *h_name; + bool malloc_h_name; +}; + static const struct gaih_servtuple nullserv; @@ -198,7 +204,8 @@ static bool convert_hostent_to_gaih_addrtuple (const struct addrinfo *req, int family, struct hostent *h, - struct gaih_addrtuple **result) + struct gaih_addrtuple **result, + struct gaih_result *res) { while (*result) result = &(*result)->next; @@ -217,6 +224,16 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req, if (array == NULL) return false; + /* Duplicate h_name because it may get reclaimed when the underlying storage + is freed. */ + if (res->h_name == NULL) + { + res->h_name = __strdup (h->h_name); + res->malloc_h_name = true; + if (res->h_name == NULL) + return false; + } + for (size_t i = 0; i < count; ++i) { if (family == AF_INET && req->ai_family == AF_INET6) @@ -233,7 +250,6 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req, } array[i].next = array + i + 1; } - array[0].name = h->h_name; array[count - 1].next = NULL; *result = array; @@ -278,7 +294,7 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req, } \ else if (status == NSS_STATUS_SUCCESS) \ { \ - if (!convert_hostent_to_gaih_addrtuple (req, _family, &th, &addrmem)) \ + if (!convert_hostent_to_gaih_addrtuple (req, _family, &th, &addrmem, &res)) \ { \ __resolv_context_enable_inet6 (res_ctx, res_enable_inet6); \ __resolv_context_put (res_ctx); \ @@ -322,14 +338,14 @@ typedef enum nss_status (*nss_getcanonname_r) memory allocation failure. The returned string is allocated on the heap; the caller has to free it. */ static char * -getcanonname (service_user *nip, struct gaih_addrtuple *at, const char *name) +getcanonname (service_user *nip, const char *hname, const char *name) { nss_getcanonname_r cfct = __nss_lookup_function (nip, "getcanonname_r"); char *s = (char *) name; if (cfct != NULL) { char buf[256]; - if (DL_CALL_FCT (cfct, (at->name ?: name, buf, sizeof (buf), + if (DL_CALL_FCT (cfct, (hname ?: name, buf, sizeof (buf), &s, &errno, &h_errno)) != NSS_STATUS_SUCCESS) /* If the canonical name cannot be determined, use the passed string. */ @@ -350,6 +366,8 @@ gaih_inet (const char *name, const struct gaih_service *service, const char *canon = NULL; const char *orig_name = name; + struct gaih_result res = {0}; + /* Reserve stack memory for the scratch buffer in the getaddrinfo function. */ size_t alloca_used = sizeof (struct scratch_buffer); @@ -590,7 +608,7 @@ gaih_inet (const char *name, const struct gaih_service *service, { /* We found data, convert it. */ if (!convert_hostent_to_gaih_addrtuple - (req, AF_INET, h, &addrmem)) + (req, AF_INET, h, &addrmem, &res)) { result = -EAI_MEMORY; goto free_and_return; @@ -883,7 +901,7 @@ gaih_inet (const char *name, const struct gaih_service *service, if ((req->ai_flags & AI_CANONNAME) != 0 && canon == NULL) { - canonbuf = getcanonname (nip, at, name); + canonbuf = getcanonname (nip, res.h_name, name); if (canonbuf == NULL) { __resolv_context_enable_inet6 @@ -1130,6 +1148,10 @@ gaih_inet (const char *name, const struct gaih_service *service, free (addrmem); free (canonbuf); + if (res.malloc_h_name){ + free (res.h_name); + } + return result; } -- 2.23.0