kern/96840: [patch] getgrent() does not return large groups via NIS

Kirk Webb kwebb at flux.utah.edu
Fri May 5 22:40:16 UTC 2006


>Number:         96840
>Category:       kern
>Synopsis:       [patch] getgrent() does not return large groups via NIS
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Fri May 05 22:40:14 GMT 2006
>Closed-Date:
>Last-Modified:
>Originator:     Kirk Webb <kwebb at flux.utah.edu>
>Release:        FreeBSD 6.0-RELEASE-p6 i386
>Organization:
University of Utah
>Environment:
System: FreeBSD vodka 6.0-RELEASE-p6 FreeBSD 6.0-RELEASE-p6 #0: Fri Apr 7 15:13:12 MDT 2006 root at vodka:/usr/obj/z/src/sys/DESKTOP i386

>Description:

The nis_groups() function in src/lib/libc/gen/getgrent.c skips groups
with a large number of members and/or large number of total characters
in the membership list when fetching from NIS.  Thus, anything using
or vectoring through getgrent() (e.g. initgroups() and getgrouplist())
will not see these groups.  getgrent_r is also affected, although the
caller may pass in a larger buffer and so avoid the problem.  The most
obviously problematic side-effect of this behavior is that users end
up with groups missing from their groups list, and so have
reduced/incorrect permissions.

>How-To-Repeat:

Add something like the following group line to an NIS server with a
FreeBSD client, with a legitimate user placed somewhere in the list
(just make sure the member list is long):

footest:*:6666:f1,f2,f3,f4,f5,f6,f7,f8,f9,f10,f11,f12,f13,f14,f15,f16,f17,f18,f19,f20,f21,f22,f23,f24,f25,f26,f27,f28,f29,f30,f31,f32,f33,f34,f35,f36,f37,f38,f39,f40,f41,f42,f43,f44,f45,f46,f47,f48,f49,f50,f51,f52,f53,f54,f55,f56,f57,f58,f59,f60,f61,f62,f63,f64,f65,f66,f67,f68,f69,f70,f71,f72,f73,f74,f75,f76,f77,f78,f79,f80,f81,f82,f83,f84,f85,f86,f87,f88,f89,f90,f91,f92,f93,f94,f95,f96,f97,f98,f99,f100,f101,f102,f103,f104,f105,f106,f107,f108,f109,f110,f111,f112,f113,f114,f115,f116,f117,f118,f119,f120,f121,f122,f123,f124,f125,f126,f127,f128,f129

Next, go to a FreeBSD 6 client bound to this server and execute
'groups' or 'id' for the user.  The test list should be missing.

Now remove half or so of the above entries, but keep the legit user,
and rebuild the server's yp database.  Execute 'groups' or 'id' against the
user on the FreeBSD 6 client and the test group should show up.

>Fix:

The included patch should do the trick.  The problem is that
nis_group() does not check for an ERANGE error code when coming back
from __gr_parse_entry().  Rather, it just summarily continues on
through the loop and tries to grab the next entry.

This patch works by saving the old NIS key in the NIS state structure
rather than replacing it immediately with the new key.  The new key is
saved off so long as an ERANGE is not encountered by
__gr_parse_entr().  If this happens, the "erange" trapdoor is taken
out of nis_group().  I tried to be careful with memory
allocation/dealloc, but a careful scan from a second (or more) set of
eyes is always a good idea.

The patch below should apply cleanly to the HEAD for file 
src/lib/libc/getgrent.c

--- getgrent.c.diff begins here ---
--- getgrent.c	Fri May  5 15:46:28 2006
+++ /usr/src/lib/libc/gen/getgrent.c	Fri May  5 15:44:13 2006
@@ -967,7 +967,6 @@
 	int		*errnop, keylen, resultlen, rv;
 	
 	name = NULL;
-        key  = NULL;
 	gid = (gid_t)-1;
 	how = (enum nss_lookup_type)mdata;
 	switch (how) {
@@ -1017,16 +1016,19 @@
 		result = NULL;
 		if (how == nss_lt_all) {
 			if (st->key == NULL)
-				rv = yp_first(st->domain, map, &key,
-				    &keylen, &result, &resultlen);
+				rv = yp_first(st->domain, map, &st->key,
+				    &st->keylen, &result, &resultlen);
 			else {
-				rv = yp_next(st->domain, map, 
-                                    st->key, st->keylen,
-                                    &key, &keylen, &result, &resultlen);
+				key = st->key;
+				keylen = st->keylen;
+				st->key = NULL;
+				rv = yp_next(st->domain, map, key, keylen,
+				    &st->key, &st->keylen, &result,
+				    &resultlen);
+				free(key);
 			}
 			if (rv != 0) {
 				free(result);
-                                free(key);
 				free(st->key);
 				st->key = NULL;
 				if (rv == YPERR_NOMORE) {
@@ -1052,35 +1054,16 @@
 		 * terminator, alignment padding, and one (char *)
 		 * pointer for the member list terminator.
 		 */
-		if (resultlen >= bufsize - _ALIGNBYTES - sizeof(char *)) {
-                        if (how == nss_lt_all) {
-                                free(key);
-                        }
-                        goto erange;
-                }
+		if (resultlen >= bufsize - _ALIGNBYTES - sizeof(char *))
+			goto erange;
 		memcpy(buffer, result, resultlen);
 		buffer[resultlen] = '\0';
 		free(result);
 		rv = __gr_match_entry(buffer, resultlen, how, name, gid);
-		if (rv == NS_SUCCESS) {
+		if (rv == NS_SUCCESS)
 			rv = __gr_parse_entry(buffer, resultlen, grp,
 			    &buffer[resultlen+1], bufsize - resultlen - 1,
 			    errnop);
-                        if (*errnop == ERANGE) {
-                                if (how == nss_lt_all) {
-                                        free(key);
-                                }
-                                goto erange;
-                        }
-                }
-                if (how == nss_lt_all) {
-                        if (st->key != NULL) {
-                                free(st->key);
-                        }
-                        st->key = key;
-                        st->keylen = keylen;
-                        key = NULL;
-                }
 	} while (how == nss_lt_all && !(rv & NS_TERMINATE));
 fin:
 	if (rv == NS_SUCCESS && retval != NULL)
--- getgrent.c.diff ends here ---


>Release-Note:
>Audit-Trail:
>Unformatted:


More information about the freebsd-bugs mailing list