diff --git a/sys/rpc/authunix_prot.c b/sys/rpc/authunix_prot.c --- a/sys/rpc/authunix_prot.c +++ b/sys/rpc/authunix_prot.c @@ -50,15 +50,18 @@ #include -/* gids compose part of a credential; there may not be more than 16 of them */ -#define NGRPS 16 - /* * XDR for unix authentication parameters. */ bool_t xdr_authunix_parms(XDR *xdrs, uint32_t *time, struct xucred *cred) { + /* Includes the effective GID. */ + const int max_ngroups = imin( + XU_NGROUPS, /* 'struct xucred', cr_groups[] field. */ + ngroups_max + 1); /* 'ngroups_max' limits supplementary groups. */ + _Static_assert(XU_NGROUPS >= 1, + "Room needed for the effective GID."); uint32_t namelen; uint32_t ngroups, i; uint32_t junk; @@ -72,13 +75,10 @@ namelen = strlen(hostbuf); if (namelen > 255) namelen = 255; - } else { + } else namelen = 0; - } - junk = 0; - if (!xdr_uint32_t(xdrs, time) - || !xdr_uint32_t(xdrs, &namelen)) + if (!xdr_uint32_t(xdrs, time) || !xdr_uint32_t(xdrs, &namelen)) return (FALSE); /* @@ -87,9 +87,8 @@ if (xdrs->x_op == XDR_ENCODE) { if (!xdr_opaque(xdrs, hostbuf, namelen)) return (FALSE); - } else { + } else xdr_setpos(xdrs, xdr_getpos(xdrs) + RNDUP(namelen)); - } if (!xdr_uint32_t(xdrs, &cred->cr_uid)) return (FALSE); @@ -98,33 +97,45 @@ 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'. */ - ngroups = cred->cr_ngroups - 1; - if (ngroups > NGRPS) - ngroups = NGRPS; + ngroups = cred->cr_ngroups; + if (ngroups > max_ngroups) + ngroups = max_ngroups; + /* The effective GID has already been treated above. */ + --ngroups; } if (!xdr_uint32_t(xdrs, &ngroups)) return (FALSE); - for (i = 0; i < 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 (ngroups > ngroups_max) - cred->cr_ngroups = ngroups_max + 1; - else - cred->cr_ngroups = ngroups + 1; - } + /* Include the effective GID back. */ + ++ngroups; + /* Handle wrap-around, causing rejection. */ + if (ngroups == 0) + --ngroups; + + MPASS(ngroups <= max_ngroups || xdrs->x_op == XDR_DECODE); + + /* + * As a DoS prevention measure, we don't accept to receive more than + * 1 effective GID + 'ngroups_max' supplementary groups. However, we + * accept receiving more than 'max_ngroups' (taking into account + * XU_NGROUPS), silently discarding the extra groups. + */ + if (ngroups > 1 + ngroups_max) + return (FALSE); + + junk = 0; + for (i = 0; i < ngroups - 1; i++) + if (!xdr_uint32_t(xdrs, + i < max_ngroups ? &cred->cr_sgroups[i] : &junk)) + return (FALSE); + + if (xdrs->x_op == XDR_DECODE) + cred->cr_ngroups = imin(max_ngroups, ngroups); return (TRUE); } diff --git a/sys/rpc/svc_auth_unix.c b/sys/rpc/svc_auth_unix.c --- a/sys/rpc/svc_auth_unix.c +++ b/sys/rpc/svc_auth_unix.c @@ -51,7 +51,6 @@ #include #define MAX_MACHINE_NAME 255 -#define NGRPS 16 /* * Unix longhand authenticator @@ -65,7 +64,7 @@ uint32_t time; struct xucred *xcr; u_int auth_len; - size_t str_len, gid_len; + size_t str_len, gid_len, ngroups, max_ngroups; u_int i; xcr = rqst->rq_clntcred; @@ -74,51 +73,64 @@ XDR_DECODE); buf = XDR_INLINE(&xdrs, auth_len); if (buf != NULL) { + /* Accounts for 'time', 'str_len', UID, GID and 'gid_len'. */ + const u_int min_len = 5 * BYTES_PER_XDR_UNIT; + if (auth_len < min_len) { + (void)printf("auth_unix: Response too short (%u)\n", auth_len); + goto badcred; + } time = IXDR_GET_UINT32(buf); str_len = (size_t)IXDR_GET_UINT32(buf); - if (str_len > MAX_MACHINE_NAME) { - stat = AUTH_BADCRED; - goto done; - } + if (str_len > MAX_MACHINE_NAME) + goto badcred; str_len = RNDUP(str_len); + /* + * Recheck message length now that we know 'str_len' to protect + * access to the credentials part. + */ + if (auth_len < min_len + str_len) { + (void)printf("auth_unix: Inconsistent response and " + "machine name lengths (%u, %zu)\n", + auth_len, str_len); + goto badcred; + } buf += str_len / sizeof (int32_t); xcr->cr_uid = IXDR_GET_UINT32(buf); xcr->cr_gid = IXDR_GET_UINT32(buf); gid_len = (size_t)IXDR_GET_UINT32(buf); - if (gid_len > NGRPS) { - stat = AUTH_BADCRED; - goto done; + /* + * Final message length check, as we now know how much we will + * read in total. + */ + if (auth_len < min_len + str_len + gid_len * BYTES_PER_XDR_UNIT) { + (void)printf("auth_unix: Inconsistent lengths " + "(response %u, machine name %zu, " + "supplementary groups %zu)\n", + auth_len, str_len, gid_len); + goto badcred; } - for (i = 0; i < gid_len; i++) { - /* - * Note that this is a `struct xucred`, which maintains - * its historical layout of preserving the egid in - * cr_ngroups and cr_groups[0] == egid. - */ - if (i + 1 < XU_NGROUPS) - xcr->cr_groups[i + 1] = IXDR_GET_INT32(buf); + /* + * 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'. + */ + max_ngroups = imin(XU_NGROUPS, ngroups_max + 1); + ngroups = gid_len + 1; + /* Handle wrap-around, causing rejection. */ + if (ngroups == 0) + --ngroups; + /* See comment before the same test in xdr_authunix_parms(). */ + if (ngroups > 1 + ngroups_max) + goto badcred; + for (i = 0; i < ngroups - 1; i++) { + if (i < max_ngroups) + xcr->cr_sgroups[i] = IXDR_GET_INT32(buf); else buf++; } - if (gid_len + 1 > XU_NGROUPS) - xcr->cr_ngroups = XU_NGROUPS; - else - xcr->cr_ngroups = gid_len + 1; - - /* - * five is the smallest unix credentials structure - - * timestamp, hostname len (0), uid, gid, and gids len (0). - */ - if ((5 + gid_len) * BYTES_PER_XDR_UNIT + str_len > auth_len) { - (void) printf("bad auth_len gid %ld str %ld auth %u\n", - (long)gid_len, (long)str_len, auth_len); - stat = AUTH_BADCRED; - goto done; - } - } else if (! xdr_authunix_parms(&xdrs, &time, xcr)) { - stat = AUTH_BADCRED; - goto done; - } + xcr->cr_ngroups = imin(max_ngroups, ngroups); + } else if (! xdr_authunix_parms(&xdrs, &time, xcr)) + goto badcred; rqst->rq_verf = _null_auth; stat = AUTH_OK; @@ -126,6 +138,9 @@ XDR_DESTROY(&xdrs); return (stat); +badcred: + stat = AUTH_BADCRED; + goto done; }