Page MenuHomeFreeBSD
Files F148812615

D52263.id161229.diff
No OneTemporary
Actions

D52263.id161229.diff
View Options

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 <rpc/rpc_com.h>
-/* 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 <rpc/rpc_com.h>
#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;
}

File Metadata

Mime Type
text/plain
Expires
Sat, Mar 21, 8:24 AM (13 h, 45 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
30014411
Default Alt Text
D52263.id161229.diff (6 KB)

Event Timeline