git: 5e24f53a2d5d - stable/13 - cred: groupmember() and co.: Sanity check cred's groups (INVARIANTS)

From: Olivier Certner <olce_at_FreeBSD.org>
Date: Fri, 15 Nov 2024 13:00:51 UTC
The branch stable/13 has been updated by olce:

URL: https://cgit.FreeBSD.org/src/commit/?id=5e24f53a2d5d3dc7d695ba0a037c1ba66ec75ba9

commit 5e24f53a2d5d3dc7d695ba0a037c1ba66ec75ba9
Author:     Olivier Certner <olce@FreeBSD.org>
AuthorDate: 2024-11-01 15:11:23 +0000
Commit:     Olivier Certner <olce@FreeBSD.org>
CommitDate: 2024-11-15 12:59:09 +0000

    cred: groupmember() and co.: Sanity check cred's groups (INVARIANTS)
    
    Leverage the normalization check functions introduced in the previous
    commit in all public-facing groups search functions to catch programming
    errors early.
    
    Approved by:    markj (mentor)
    MFC after:      3 days
    
    (cherry picked from commit 634675067867090e538b08e62ff9b14d3ffae5a3)
    
    Approved by:    markj (mentor)
---
 sys/kern/kern_prot.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c
index 6cdfea2774c0..ede79f7d6bd2 100644
--- a/sys/kern/kern_prot.c
+++ b/sys/kern/kern_prot.c
@@ -1354,6 +1354,8 @@ int
 group_is_supplementary(const gid_t gid, const struct ucred *const cred)
 {
 
+	groups_check_normalized(cred->cr_ngroups, cred->cr_groups);
+
 	/*
 	 * Perform a binary search of the supplementary groups.  This is
 	 * possible because we sort the groups in crsetgroups().
@@ -1377,7 +1379,9 @@ groupmember(gid_t gid, const struct ucred *cred)
 	if (cred->cr_ngroups == 0)
 		return (0);
 
-	if (cred->cr_groups[0] == gid)
+	groups_check_positive_len(cred->cr_ngroups);
+
+	if (gid == cred->cr_groups[0])
 		return (1);
 
 	return (group_is_supplementary(gid, cred));
@@ -1390,6 +1394,14 @@ groupmember(gid_t gid, const struct ucred *cred)
 int
 realgroupmember(gid_t gid, const struct ucred *cred)
 {
+	/*
+	 * Although the equality test on 'cr_rgid' below doesn't access
+	 * 'cr_groups', we check for the latter's length here as we assume that,
+	 * if 'cr_ngroups' is 0, the passed 'struct ucred' is invalid, and
+	 * 'cr_rgid' may not have been filled.
+	 */
+	groups_check_positive_len(cred->cr_ngroups);
+
 	if (gid == cred->cr_rgid)
 		return (1);