mgetgroups: Avoid an unlikely signed integer overflow on macOS.
Collin Funk <[email protected]>
| Newsgroups | gmane.comp.lib.gnulib.bugs |
|---|---|
| Message-ID | <[email protected]> |
I've attached 2 patches. The first avoids an unlikely (impossible?) integer overflow used for allocating a buffer. It only occurs on systems with a buggy getgrouplist. The comments say Darwin has the bug, but I haven't checked which versions. This was found in coreutils coverity. The second adjusts the module to use reallocarray, instead of implementing a similar function ourself here. Will push them in a bit. Collin
0001-mgetgroups-Avoid-an-unlikely-signed-integer-overflow.patch
(text/x-patch, 2.4 KB)
From b1f9075150b15aa3569f971b47ff8c5eba2bf7c4 Mon Sep 17 00:00:00 2001 Message-ID: <b1f9075150b15aa3569f971b47ff8c5eba2bf7c4.1786156916.git.collin.funk1@gmail.com> From: Collin Funk <[email protected]> Date: Fri, 7 Aug 2026 19:27:42 -0700 Subject: [PATCH 1/2] mgetgroups: Avoid an unlikely signed integer overflow on macOS. Problem found by Coverity (CID 1638872). * lib/mgetgroups.c: Include stdckdint.h. (mgetgroups): Check for a signed integer overflow when increasing the size of the array for reallocation. * modules/mgetgroups (Depends-on): Add stdckdint-h. --- ChangeLog | 9 +++++++++ lib/mgetgroups.c | 10 ++++++++-- modules/mgetgroups | 1 + 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index dcc91e5ba5..18722ca80f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +2026-08-07 Collin Funk <[email protected]> + + mgetgroups: Avoid an unlikely signed integer overflow on macOS. + Problem found by Coverity (CID 1638872). + * lib/mgetgroups.c: Include stdckdint.h. + (mgetgroups): Check for a signed integer overflow when increasing the + size of the array for reallocation. + * modules/mgetgroups (Depends-on): Add stdckdint-h. + 2026-08-07 Bruno Haible <[email protected]> Fix position of _GL_ATTRIBUTE_REPRODUCIBLE in last commit. diff --git a/lib/mgetgroups.c b/lib/mgetgroups.c index 0aa1eb3948..f1eea0fee2 100644 --- a/lib/mgetgroups.c +++ b/lib/mgetgroups.c @@ -23,6 +23,7 @@ #include <stdlib.h> #include <unistd.h> +#include <stdckdint.h> #include <stdint.h> #include <string.h> #include <errno.h> @@ -92,8 +93,13 @@ mgetgroups (char const *username, gid_t gid, gid_t **groups) /* Some systems (like Darwin) have a bug where they never increase max_n_groups. */ - if (ng < 0 && last_n_groups == max_n_groups) - max_n_groups *= 2; + if (ng < 0 && last_n_groups == max_n_groups + && ckd_mul (&max_n_groups, max_n_groups, 2)) + { + free (g); + errno = ENOMEM; + return -1; + } gid_t *h = realloc_groupbuf (g, max_n_groups); if (h == NULL) diff --git a/modules/mgetgroups b/modules/mgetgroups index bc8a9ddf25..edb28adf53 100644 --- a/modules/mgetgroups +++ b/modules/mgetgroups @@ -11,6 +11,7 @@ free-posix getgroups getugroups realloc-posix +stdckdint-h xalloc-oversized configure.ac: -- 2.55.0
0002-mgetgroups-Prefer-reallocarray-to-xalloc-oversized.patch
(text/x-patch, 3.9 KB)
From aa21e35122e6e4c42fdea823b6afa2eff39ee52b Mon Sep 17 00:00:00 2001 Message-ID: <aa21e35122e6e4c42fdea823b6afa2eff39ee52b.1786156916.git.collin.funk1@gmail.com> In-Reply-To: <b1f9075150b15aa3569f971b47ff8c5eba2bf7c4.1786156916.git.collin.funk1@gmail.com> References: <b1f9075150b15aa3569f971b47ff8c5eba2bf7c4.1786156916.git.collin.funk1@gmail.com> From: Collin Funk <[email protected]> Date: Fri, 7 Aug 2026 19:36:00 -0700 Subject: [PATCH 2/2] mgetgroups: Prefer reallocarray to xalloc-oversized. * lib/mgetgroups.c: Don't include xalloc-oversized.h. (realloc_groupbuf): Remove function. (mgetgroups): Use reallocarray instead of realloc_groupbuf. * modules/mgetgroups (Depends-on): Add reallocarray. Remove realloc-posix and xalloc-oversized. --- ChangeLog | 7 +++++++ lib/mgetgroups.c | 21 ++++----------------- modules/mgetgroups | 3 +-- 3 files changed, 12 insertions(+), 19 deletions(-) diff --git a/ChangeLog b/ChangeLog index 18722ca80f..839fe23c67 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,12 @@ 2026-08-07 Collin Funk <[email protected]> + mgetgroups: Prefer reallocarray to xalloc-oversized. + * lib/mgetgroups.c: Don't include xalloc-oversized.h. + (realloc_groupbuf): Remove function. + (mgetgroups): Use reallocarray instead of realloc_groupbuf. + * modules/mgetgroups (Depends-on): Add reallocarray. Remove + realloc-posix and xalloc-oversized. + mgetgroups: Avoid an unlikely signed integer overflow on macOS. Problem found by Coverity (CID 1638872). * lib/mgetgroups.c: Include stdckdint.h. diff --git a/lib/mgetgroups.c b/lib/mgetgroups.c index f1eea0fee2..5c4b14cdef 100644 --- a/lib/mgetgroups.c +++ b/lib/mgetgroups.c @@ -32,7 +32,6 @@ #endif #include "getugroups.h" -#include "xalloc-oversized.h" /* Work around an incompatibility of OS X 10.11: getgrouplist accepts int *, not gid_t *, and int and gid_t differ in sign. */ @@ -40,18 +39,6 @@ # pragma GCC diagnostic ignored "-Wpointer-sign" #endif -static gid_t * -realloc_groupbuf (gid_t *g, size_t num) -{ - if (xalloc_oversized (num, sizeof *g)) - { - errno = ENOMEM; - return NULL; - } - - return realloc (g, num * sizeof *g); -} - /* Like getugroups, but store the result in malloc'd storage. Set *GROUPS to the malloc'd list of all group IDs of which USERNAME is a member. If GID is not -1, store it first. GID should be the @@ -80,7 +67,7 @@ mgetgroups (char const *username, gid_t gid, gid_t **groups) enum { N_GROUPS_INIT = 10 }; int max_n_groups = N_GROUPS_INIT; - gid_t *g = realloc_groupbuf (NULL, max_n_groups); + gid_t *g = reallocarray (NULL, max_n_groups, sizeof *g); if (g == NULL) return -1; @@ -101,7 +88,7 @@ mgetgroups (char const *username, gid_t gid, gid_t **groups) return -1; } - gid_t *h = realloc_groupbuf (g, max_n_groups); + gid_t *h = reallocarray (g, max_n_groups, sizeof *g); if (h == NULL) { free (g); @@ -132,7 +119,7 @@ mgetgroups (char const *username, gid_t gid, gid_t **groups) { if (errno == ENOSYS) { - gid_t *g = realloc_groupbuf (NULL, 1); + gid_t *g = reallocarray (NULL, 1, sizeof *g); if (g) { *groups = g; @@ -145,7 +132,7 @@ mgetgroups (char const *username, gid_t gid, gid_t **groups) if (max_n_groups == 0 || (!username && gid != (gid_t) -1)) max_n_groups++; - gid_t *g = realloc_groupbuf (NULL, max_n_groups); + gid_t *g = reallocarray (NULL, max_n_groups, sizeof *g); if (g == NULL) return -1; diff --git a/modules/mgetgroups b/modules/mgetgroups index edb28adf53..01754b25bb 100644 --- a/modules/mgetgroups +++ b/modules/mgetgroups @@ -10,9 +10,8 @@ Depends-on: free-posix getgroups getugroups -realloc-posix +reallocarray stdckdint-h -xalloc-oversized configure.ac: gl_MGETGROUPS -- 2.55.0