rpms/glibc
7
0
Fork 1
Code Issues Pull Requests Releases Activity

CVE-2024-33601, CVE-2024-33602: nscd: netgroup: Use two buffers in addgetnetgrentX

Browse Source 
Resolves: RHEL-34272
Resolves: RHEL-34274
This commit is contained in:
Patsy Griffin 2024-06-13 22:25:57 -04:00
parent ec53b9158c
commit d3837bb611
3 changed files with 421 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

382
glibc-RHEL-34272-1.patch Normal file
View File

@ -0,0 +1,382 @@
commit c04a21e050d64a1193a6daab872bca2528bda44b
Author: Florian Weimer <fweimer@redhat.com>
Date: Thu Apr 25 15:01:07 2024 +0200
CVE-2024-33601, CVE-2024-33602: nscd: netgroup: Use two buffers in addgetnetgrentX (bug 31680)
This avoids potential memory corruption when the underlying NSS
callback function does not use the buffer space to store all strings
(e.g., for constant strings).
Instead of custom buffer management, two scratch buffers are used.
This increases stack usage somewhat.
Scratch buffer allocation failure is handled by return -1
(an invalid timeout value) instead of terminating the process.
This fixes bug 31679.
Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
diff -Nrup glibc-2.34/nscd/netgroupcache.c b/nscd/netgroupcache.c
--- glibc-2.34/nscd/netgroupcache.c 2024-06-13 13:55:16.406511485 -0400
+++ b/nscd/netgroupcache.c 2024-06-13 13:34:45.523345077 -0400
@@ -24,6 +24,7 @@
#include <stdlib.h>
#include <unistd.h>
#include <sys/mman.h>
+#include <scratch_buffer.h>
#include "../inet/netgroup.h"
#include "nscd.h"
@@ -66,6 +67,16 @@ struct dataset
char strdata[0];
};
+/* Send a notfound response to FD. Always returns -1 to indicate an
+ ephemeral error. */
+static time_t
+send_notfound (int fd)
+{
+ if (fd != -1)
+ TEMP_FAILURE_RETRY (send (fd, &notfound, sizeof (notfound), MSG_NOSIGNAL));
+ return -1;
+}
+
/* Sends a notfound message and prepares a notfound dataset to write to the
cache. Returns true if there was enough memory to allocate the dataset and
returns the dataset in DATASETP, total bytes to write in TOTALP and the
@@ -84,8 +95,7 @@ do_notfound (struct database_dyn *db, in
total = sizeof (notfound);
timeout = time (NULL) + db->negtimeout;
- if (fd != -1)
- TEMP_FAILURE_RETRY (send (fd, &notfound, total, MSG_NOSIGNAL));
+ send_notfound (fd);
dataset = mempool_alloc (db, sizeof (struct dataset) + req->key_len, 1);
/* If we cannot permanently store the result, so be it. */
@@ -110,11 +120,78 @@ do_notfound (struct database_dyn *db, in
return cacheable;
}
+struct addgetnetgrentX_scratch
+{
+ /* This is the result that the caller should use. It can be NULL,
+ point into buffer, or it can be in the cache. */
+ struct dataset *dataset;
+
+ struct scratch_buffer buffer;
+
+ /* Used internally in addgetnetgrentX as a staging area. */
+ struct scratch_buffer tmp;
+
+ /* Number of bytes in buffer that are actually used. */
+ size_t buffer_used;
+};
+
+static void
+addgetnetgrentX_scratch_init (struct addgetnetgrentX_scratch *scratch)
+{
+ scratch->dataset = NULL;
+ scratch_buffer_init (&scratch->buffer);
+ scratch_buffer_init (&scratch->tmp);
+
+ /* Reserve space for the header. */
+ scratch->buffer_used = sizeof (struct dataset);
+ static_assert (sizeof (struct dataset) < sizeof (scratch->tmp.__space),
+ "initial buffer space");
+ memset (scratch->tmp.data, 0, sizeof (struct dataset));
+}
+
+static void
+addgetnetgrentX_scratch_free (struct addgetnetgrentX_scratch *scratch)
+{
+ scratch_buffer_free (&scratch->buffer);
+ scratch_buffer_free (&scratch->tmp);
+}
+
+/* Copy LENGTH bytes from S into SCRATCH. Returns NULL if SCRATCH
+ could not be resized, otherwise a pointer to the copy. */
+static char *
+addgetnetgrentX_append_n (struct addgetnetgrentX_scratch *scratch,
+ const char *s, size_t length)
+{
+ while (true)
+ {
+ size_t remaining = scratch->buffer.length - scratch->buffer_used;
+ if (remaining >= length)
+ break;
+ if (!scratch_buffer_grow_preserve (&scratch->buffer))
+ return NULL;
+ }
+ char *copy = scratch->buffer.data + scratch->buffer_used;
+ memcpy (copy, s, length);
+ scratch->buffer_used += length;
+ return copy;
+}
+
+/* Copy S into SCRATCH, including its null terminator. Returns false
+ if SCRATCH could not be resized. */
+static bool
+addgetnetgrentX_append (struct addgetnetgrentX_scratch *scratch, const char *s)
+{
+ if (s == NULL)
+ s = "";
+ return addgetnetgrentX_append_n (scratch, s, strlen (s) + 1) != NULL;
+}
+
+/* Caller must initialize and free *SCRATCH. If the return value is
+ negative, this function has sent a notfound response. */
static time_t
addgetnetgrentX (struct database_dyn *db, int fd, request_header *req,
const char *key, uid_t uid, struct hashentry *he,
- struct datahead *dh, struct dataset **resultp,
- void **tofreep)
+ struct datahead *dh, struct addgetnetgrentX_scratch *scratch)
{
if (__glibc_unlikely (debug_level > 0))
{
@@ -133,14 +210,10 @@ addgetnetgrentX (struct database_dyn *db
char *key_copy = NULL;
struct __netgrent data;
- size_t buflen = MAX (1024, sizeof (*dataset) + req->key_len);
- size_t buffilled = sizeof (*dataset);
- char *buffer = NULL;
size_t nentries = 0;
size_t group_len = strlen (key) + 1;
struct name_list *first_needed
= alloca (sizeof (struct name_list) + group_len);
- *tofreep = NULL;
if (netgroup_database == NULL
&& !__nss_database_get (nss_database_netgroup, &netgroup_database))
@@ -152,8 +225,6 @@ addgetnetgrentX (struct database_dyn *db
}
memset (&data, '\0', sizeof (data));
- buffer = xmalloc (buflen);
- *tofreep = buffer;
first_needed->next = first_needed;
memcpy (first_needed->name, key, group_len);
data.needed_groups = first_needed;
@@ -196,8 +267,8 @@ addgetnetgrentX (struct database_dyn *db
while (1)
{
int e;
- status = getfct.f (&data, buffer + buffilled,
- buflen - buffilled - req->key_len, &e);
+ status = getfct.f (&data, scratch->tmp.data,
+ scratch->tmp.length, &e);
if (status == NSS_STATUS_SUCCESS)
{
if (data.type == triple_val)
@@ -205,68 +276,10 @@ addgetnetgrentX (struct database_dyn *db
const char *nhost = data.val.triple.host;
const char *nuser = data.val.triple.user;
const char *ndomain = data.val.triple.domain;
-
- size_t hostlen = strlen (nhost ?: "") + 1;
- size_t userlen = strlen (nuser ?: "") + 1;
- size_t domainlen = strlen (ndomain ?: "") + 1;
-
- if (nhost == NULL || nuser == NULL || ndomain == NULL
- || nhost > nuser || nuser > ndomain)
- {
- const char *last = nhost;
- if (last == NULL
- || (nuser != NULL && nuser > last))
- last = nuser;
- if (last == NULL
- || (ndomain != NULL && ndomain > last))
- last = ndomain;
-
- size_t bufused
- = (last == NULL
- ? buffilled
- : last + strlen (last) + 1 - buffer);
-
- /* We have to make temporary copies. */
- size_t needed = hostlen + userlen + domainlen;
-
- if (buflen - req->key_len - bufused < needed)
- {
- buflen += MAX (buflen, 2 * needed);
- /* Save offset in the old buffer. We don't
- bother with the NULL check here since
- we'll do that later anyway. */
- size_t nhostdiff = nhost - buffer;
- size_t nuserdiff = nuser - buffer;
- size_t ndomaindiff = ndomain - buffer;
-
- char *newbuf = xrealloc (buffer, buflen);
- /* Fix up the triplet pointers into the new
- buffer. */
- nhost = (nhost ? newbuf + nhostdiff
- : NULL);
- nuser = (nuser ? newbuf + nuserdiff
- : NULL);
- ndomain = (ndomain ? newbuf + ndomaindiff
- : NULL);
- *tofreep = buffer = newbuf;
- }
-
- nhost = memcpy (buffer + bufused,
- nhost ?: "", hostlen);
- nuser = memcpy ((char *) nhost + hostlen,
- nuser ?: "", userlen);
- ndomain = memcpy ((char *) nuser + userlen,
- ndomain ?: "", domainlen);
- }
-
- char *wp = buffer + buffilled;
- wp = memmove (wp, nhost ?: "", hostlen);
- wp += hostlen;
- wp = memmove (wp, nuser ?: "", userlen);
- wp += userlen;
- wp = memmove (wp, ndomain ?: "", domainlen);
- wp += domainlen;
- buffilled = wp - buffer;
+ if (!(addgetnetgrentX_append (scratch, nhost)
+ && addgetnetgrentX_append (scratch, nuser)
+ && addgetnetgrentX_append (scratch, ndomain)))
+ return send_notfound (fd);
++nentries;
}
else
@@ -318,8 +331,8 @@ addgetnetgrentX (struct database_dyn *db
}
else if (status == NSS_STATUS_TRYAGAIN && e == ERANGE)
{
- buflen *= 2;
- *tofreep = buffer = xrealloc (buffer, buflen);
+ if (!scratch_buffer_grow (&scratch->tmp))
+ return send_notfound (fd);
}
else if (status == NSS_STATUS_RETURN
|| status == NSS_STATUS_NOTFOUND
@@ -352,10 +365,17 @@ addgetnetgrentX (struct database_dyn *db
goto maybe_cache_add;
}
- total = buffilled;
+ /* Capture the result size without the key appended. */
+ total = scratch->buffer_used;
+
+ /* Make a copy of the key. The scratch buffer must not move after
+ this point. */
+ key_copy = addgetnetgrentX_append_n (scratch, key, req->key_len);
+ if (key_copy == NULL)
+ return send_notfound (fd);
/* Fill in the dataset. */
- dataset = (struct dataset *) buffer;
+ dataset = scratch->buffer.data;
timeout = datahead_init_pos (&dataset->head, total + req->key_len,
total - offsetof (struct dataset, resp),
he == NULL ? 0 : dh->nreloads + 1,
@@ -364,11 +384,7 @@ addgetnetgrentX (struct database_dyn *db
dataset->resp.version = NSCD_VERSION;
dataset->resp.found = 1;
dataset->resp.nresults = nentries;
- dataset->resp.result_len = buffilled - sizeof (*dataset);
-
- assert (buflen - buffilled >= req->key_len);
- key_copy = memcpy (buffer + buffilled, key, req->key_len);
- buffilled += req->key_len;
+ dataset->resp.result_len = total - sizeof (*dataset);
/* Now we can determine whether on refill we have to create a new
record or not. */
@@ -399,7 +415,7 @@ addgetnetgrentX (struct database_dyn *db
if (__glibc_likely (newp != NULL))
{
/* Adjust pointer into the memory block. */
- key_copy = (char *) newp + (key_copy - buffer);
+ key_copy = (char *) newp + (key_copy - (char *) dataset);
dataset = memcpy (newp, dataset, total + req->key_len);
cacheable = true;
@@ -440,7 +456,7 @@ addgetnetgrentX (struct database_dyn *db
}
out:
- *resultp = dataset;
+ scratch->dataset = dataset;
return timeout;
}
@@ -461,6 +477,9 @@ addinnetgrX (struct database_dyn *db, in
if (user != NULL)
key = (char *) rawmemchr (key, '\0') + 1;
const char *domain = *key++ ? key : NULL;
+ struct addgetnetgrentX_scratch scratch;
+
+ addgetnetgrentX_scratch_init (&scratch);
if (__glibc_unlikely (debug_level > 0))
{
@@ -476,12 +495,8 @@ addinnetgrX (struct database_dyn *db, in
group, group_len,
db, uid);
time_t timeout;
- void *tofree;
if (result != NULL)
- {
- timeout = result->head.timeout;
- tofree = NULL;
- }
+ timeout = result->head.timeout;
else
{
request_header req_get =
@@ -490,7 +505,10 @@ addinnetgrX (struct database_dyn *db, in
.key_len = group_len
};
timeout = addgetnetgrentX (db, -1, &req_get, group, uid, NULL, NULL,
- &result, &tofree);
+ &scratch);
+ result = scratch.dataset;
+ if (timeout < 0)
+ goto out;
}
struct indataset
@@ -604,7 +622,7 @@ addinnetgrX (struct database_dyn *db, in
}
out:
- free (tofree);
+ addgetnetgrentX_scratch_free (&scratch);
return timeout;
}
@@ -614,11 +632,12 @@ addgetnetgrentX_ignore (struct database_
const char *key, uid_t uid, struct hashentry *he,
struct datahead *dh)
{
- struct dataset *ignore;
- void *tofree;
- time_t timeout = addgetnetgrentX (db, fd, req, key, uid, he, dh,
- &ignore, &tofree);
- free (tofree);
+ struct addgetnetgrentX_scratch scratch;
+ addgetnetgrentX_scratch_init (&scratch);
+ time_t timeout = addgetnetgrentX (db, fd, req, key, uid, he, dh, &scratch);
+ addgetnetgrentX_scratch_free (&scratch);
+ if (timeout < 0)
+ timeout = 0;
return timeout;
}
@@ -662,5 +681,9 @@ readdinnetgr (struct database_dyn *db, s
.key_len = he->len
};
- return addinnetgrX (db, -1, &req, db->data + he->key, he->owner, he, dh);
+ int timeout = addinnetgrX (db, -1, &req, db->data + he->key, he->owner,
+ he, dh);
+ if (timeout < 0)
+ timeout = 0;
+ return timeout;
}

30
glibc-RHEL-34272-2.patch Normal file
View File

@ -0,0 +1,30 @@
commit 4bbca1a44691a6e9adcee5c6798a707b626bc331
Author: Florian Weimer <fweimer@redhat.com>
Date: Thu May 2 17:06:19 2024 +0200
nscd: Use time_t for return type of addgetnetgrentX
Using int may give false results for future dates (timeouts after the
year 2028).
Fixes commit 04a21e050d64a1193a6daab872bca2528bda44b ("CVE-2024-33601,
CVE-2024-33602: nscd: netgroup: Use two buffers in addgetnetgrentX
(bug 31680)").
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
diff --git a/nscd/netgroupcache.c b/nscd/netgroupcache.c
index 4b35498e3f..5fdcf4204e 100644
--- a/nscd/netgroupcache.c
+++ b/nscd/netgroupcache.c
@@ -680,8 +680,8 @@ readdinnetgr (struct database_dyn *db, struct hashentry *he,
.key_len = he->len
};
- int timeout = addinnetgrX (db, -1, &req, db->data + he->key, he->owner,
- he, dh);
+ time_t timeout = addinnetgrX (db, -1, &req, db->data + he->key, he->owner,
+ he, dh);
if (timeout < 0)
timeout = 0;
return timeout;

10
glibc.spec
View File

@ -155,7 +155,7 @@ end \
Summary: The GNU libc libraries
Name: glibc
Version: %{glibcversion}
Release: 112%{?dist}
Release: 113%{?dist}
# In general, GPLv2+ is used by programs, LGPLv2+ is used for
# libraries.
@ -830,6 +830,8 @@ Patch593: glibc-RHEL-25063.patch
Patch594: glibc-RHEL-34265.patch
Patch595: glibc-RHEL-34268-1.patch
Patch596: glibc-RHEL-34268-2.patch
Patch597: glibc-RHEL-34272-1.patch
Patch598: glibc-RHEL-34272-2.patch
##############################################################################
# Continued list of core "glibc" package information:
@ -2988,6 +2990,12 @@ update_gconv_modules_cache ()
%endif
%changelog
* Thu Jun 13 2024 Patsy Griffin <patsy@redhat.com> - 2.34-113
- CVE-2024-33601 glibc: netgroup cache may terminate daemon on
memory allocation failure (RHEL-34272)
- CVE-2024-33602 glibc: netgroup cache assumes NSS callback
uses in-buffer strings (RHEL-34274)
* Tue Jun 11 2024 Patsy Griffin <patsy@redhat.com> - 2.34-112
- CVE-2024-33600: nscd: Avoid null pointer crashes after notfound
response (RHEL-34268)