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 9f1cde27..07fbf441 100644 --- a/sysdeps/posix/getaddrinfo.c +++ b/sysdeps/posix/getaddrinfo.c @@ -106,6 +106,12 @@ struct gaih_servtuple int port; }; +struct gaih_result +{ + char *h_name; + bool malloc_h_name; +}; + static const struct gaih_servtuple nullserv; @@ -197,7 +203,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; @@ -216,6 +223,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) @@ -232,7 +249,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; @@ -275,7 +291,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_put (res_ctx); \ result = -EAI_SYSTEM; \ @@ -307,14 +323,14 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req, memory allocation failure. The returned string is allocated on the heap; the caller has to free it. */ static char * -getcanonname (nss_action_list nip, struct gaih_addrtuple *at, const char *name) +getcanonname (nss_action_list 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. */ @@ -335,6 +351,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); @@ -574,7 +592,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; @@ -858,7 +876,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_put (res_ctx); @@ -1101,6 +1119,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.33.0