Ignore extra groups that don't fit in the buffer passed to getgrouplist(3)

Our kernel supports 16 groups (NGROUPS_MAX), but nothing prevents
an admin from adding a user to more groups.  With that tweak we'll keep
on ignoring them instead of potentially reading past the buffer passed to
getgrouplist(3).  That behavior is explicitely described in initgroups(3).

ok millert@ gilles@

2 files changed, 23 insertions, 11 deletions

diff --git a/usr.bin/id/id.c b/usr.bin/id/id.c
index bb1ebc0e3fd..02c3d2a1e98 100644
--- a/usr.bin/id/id.c
+++ b/usr.bin/id/id.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: id.c,v 1.30 2023/05/30 16:44:16 op Exp $	*/
+/*	$OpenBSD: id.c,v 1.31 2024/11/04 21:59:15 jca Exp $	*/
 
 /*-
  * Copyright (c) 1991, 1993
@@ -269,7 +269,7 @@ void
 user(struct passwd *pw)
 {
 	gid_t gid, groups[NGROUPS_MAX + 1];
-	int cnt, ngroups;
+	int cnt, maxgroups, ngroups;
 	uid_t uid;
 	struct group *gr;
 	char *prefix;
@@ -279,8 +279,11 @@ user(struct passwd *pw)
 	(void)printf(" gid=%u", pw->pw_gid);
 	if ((gr = getgrgid(pw->pw_gid)))
 		(void)printf("(%s)", gr->gr_name);
-	ngroups = NGROUPS_MAX + 1;
-	(void) getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups);
+	maxgroups = ngroups = NGROUPS_MAX + 1;
+	if (getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups) == -1) {
+		/* Silently truncate group list */
+		ngroups = maxgroups;
+	}
 	prefix = " groups=";
 	for (cnt = 0; cnt < ngroups;) {
 		gid = groups[cnt];
@@ -298,14 +301,20 @@ user(struct passwd *pw)
 void
 group(struct passwd *pw, int nflag)
 {
-	int cnt, ngroups;
+	int cnt, maxgroups, ngroups;
 	gid_t gid, groups[NGROUPS_MAX + 1];
 	struct group *gr;
 	char *prefix;
 
 	if (pw) {
-		ngroups = NGROUPS_MAX + 1;
-		(void) getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups);
+		int ret;
+
+		maxgroups = ngroups = NGROUPS_MAX + 1;
+		ret = getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups);
+		if (ret == -1) {
+			/* Silently truncate group list */
+			ngroups = maxgroups;
+		}
 	} else {
 		groups[0] = getgid();
 		ngroups = getgroups(NGROUPS_MAX, groups + 1) + 1;
diff --git a/usr.bin/ssh/groupaccess.c b/usr.bin/ssh/groupaccess.c
index 69fd007fe15..5303688ba03 100644
--- a/usr.bin/ssh/groupaccess.c
+++ b/usr.bin/ssh/groupaccess.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: groupaccess.c,v 1.17 2019/03/06 22:14:23 dtucker Exp $ */
+/* $OpenBSD: groupaccess.c,v 1.18 2024/11/04 21:59:15 jca Exp $ */
 /*
  * Copyright (c) 2001 Kevin Steves.  All rights reserved.
  *
@@ -48,15 +48,18 @@ int
 ga_init(const char *user, gid_t base)
 {
 	gid_t groups_bygid[NGROUPS_MAX + 1];
-	int i, j;
+	int i, j, maxgroups;
 	struct group *gr;
 
 	if (ngroups > 0)
 		ga_free();
 
-	ngroups = sizeof(groups_bygid) / sizeof(gid_t);
-	if (getgrouplist(user, base, groups_bygid, &ngroups) == -1)
+	maxgroups = ngroups = sizeof(groups_bygid) / sizeof(gid_t);
+	if (getgrouplist(user, base, groups_bygid, &ngroups) == -1) {
 		logit("getgrouplist: groups list too small");
+		/* Truncate group list */
+		ngroups = maxgroups;
+	}
 	for (i = 0, j = 0; i < ngroups; i++)
 		if ((gr = getgrgid(groups_bygid[i])) != NULL)
 			groups_byname[j++] = xstrdup(gr->gr_name);