git: 47e9c81d4f13 - main - sys/rpc: UNIX auth: Fix OOB accesses, notably writes on decode

The branch main has been updated by olce:

URL: https://cgit.FreeBSD.org/src/commit/?id=47e9c81d4f1324674c624df02a51ad3a72aa7444

commit 47e9c81d4f1324674c624df02a51ad3a72aa7444
Author:     Olivier Certner <olce@FreeBSD.org>
AuthorDate: 2025-10-07 10:02:23 +0000
Commit:     Olivier Certner <olce@FreeBSD.org>
CommitDate: 2025-10-14 12:21:48 +0000

    sys/rpc: UNIX auth: Fix OOB accesses, notably writes on decode
    
    When the received authentication message had more than XU_NGROUPS, we
    would write group IDs beyond the end of cr_groups[] in the 'struct
    xucred' being filled (as 'ngroups_max' is always greater than
    XU_NGROUPS).
    
    For robustness, prevent various OOB accesses that would result from
    a change of value of XU_NGROUPS or a 'struct xucred' with an invalid
    'cr_ngroups' field, even if these cases are unlikely.
    
    Reviewed by:    rmacklem
    Fixes:          dfdcada31e79 ("Add the new kernel-mode NFS Lock Manager.")
    MFC after:      2 days
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D52960
---
 sys/rpc/authunix_prot.c | 40 +++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/sys/rpc/authunix_prot.c b/sys/rpc/authunix_prot.c
index f63a6d3f9dc6..89f0ab3ed44e 100644
--- a/sys/rpc/authunix_prot.c
+++ b/sys/rpc/authunix_prot.c
@@ -75,7 +75,6 @@ xdr_authunix_parms(XDR *xdrs, uint32_t *time, struct xucred *cred)
 	} else {
 		namelen = 0;
 	}
-	junk = 0;
 
 	if (!xdr_uint32_t(xdrs, time)
 	    || !xdr_uint32_t(xdrs, &namelen))
@@ -93,15 +92,25 @@ xdr_authunix_parms(XDR *xdrs, uint32_t *time, struct xucred *cred)
 
 	if (!xdr_uint32_t(xdrs, &cred->cr_uid))
 		return (FALSE);
+
+	/*
+	 * Safety check: The protocol needs at least one group (access to
+	 * 'cr_gid', decrementation of 'cr_ngroups' below).
+	 */
+	if (xdrs->x_op == XDR_ENCODE && cred->cr_ngroups == 0)
+		return (FALSE);
 	if (!xdr_uint32_t(xdrs, &cred->cr_gid))
 		return (FALSE);
 
 	if (xdrs->x_op == XDR_ENCODE) {
 		/*
-		 * Note that this is a `struct xucred`, which maintains its
-		 * historical layout of preserving the egid in cr_ngroups and
-		 * cr_groups[0] == egid.
+		 * Note that this is a 'struct xucred', which still has the
+		 * historical layout where the effective GID is in cr_groups[0]
+		 * and is accounted in 'cr_ngroups'.  We substract 1 to obtain
+		 * the number of "supplementary" groups, passed in the AUTH_SYS
+		 * credentials variable-length array called gids[] in RFC 5531.
 		 */
+		MPASS(cred->cr_ngroups <= XU_NGROUPS);
 		supp_ngroups = cred->cr_ngroups - 1;
 		if (supp_ngroups > NGRPS)
 			supp_ngroups = NGRPS;
@@ -109,22 +118,15 @@ xdr_authunix_parms(XDR *xdrs, uint32_t *time, struct xucred *cred)
 
 	if (!xdr_uint32_t(xdrs, &supp_ngroups))
 		return (FALSE);
-	for (i = 0; i < supp_ngroups; i++) {
-		if (i < ngroups_max) {
-			if (!xdr_uint32_t(xdrs, &cred->cr_groups[i + 1]))
-				return (FALSE);
-		} else {
-			if (!xdr_uint32_t(xdrs, &junk))
-				return (FALSE);
-		}
-	}
 
-	if (xdrs->x_op == XDR_DECODE) {
-		if (supp_ngroups > ngroups_max)
-			cred->cr_ngroups = ngroups_max + 1;
-		else
-			cred->cr_ngroups = supp_ngroups + 1;
-	}
+	junk = 0;
+	for (i = 0; i < supp_ngroups; ++i)
+		if (!xdr_uint32_t(xdrs, i < XU_NGROUPS - 1 ?
+		    &cred->cr_sgroups[i] : &junk))
+			return (FALSE);
+
+	if (xdrs->x_op != XDR_ENCODE)
+		cred->cr_ngroups = MIN(supp_ngroups + 1, XU_NGROUPS);
 
 	return (TRUE);
 }