1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
|
From 973fe93a5675c42798b2161c6f29c01b0e243994 Mon Sep 17 00:00:00 2001
From: Siddhesh Poyarekar <siddhesh@sourceware.org>
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 <siddhesh@sourceware.org>
---
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
|