Fixed a rather serious bug that presents itself when FreeBSD is configured

as an NIS client. The pw_breakout_yp routines that are used to populate the
_pw_passwd structire only do anything if the bits in the pw_fields member
_pw_passwd are cleared. Unfortunately, we can get into a state where
pw_fields has garbage in it right before the YP lookup functions are
called, which causes the breakout functions to screw up in a big way.
Here's how to duplicate the problem:

- Configure FreeBSD as an NIS client
- Log in as a user who's password database records reside only in
  the NIS passwd maps.
- Type ps -aux

Result: your processes appear to be owned by 'root' or 'deamon.'
/bin/ls can exhibit the same problem.

The reason this happens:

- When ps(1) needs to match a username to a UID, it calls getpwuid().

- root is in the local password file, so getpwuid() calls  __hashpw()
  and __hashpw() populates the _pw_passwd struct, including the pw_fields
  member. This happens before NIS lookups take place because, by coincidence,
  ps(1) tends to display processes owned by root before it happens upon
  a proccess owned by you.

- When your UID comes up, __hashpw() fails to find your entry in the
  local password database, so it bails out, BUT THE BITS IN THE pw_fields
  STRUCTURE OF _pw_passwd ARE NEVER CLEARED AND STILL CONTAIN INFORMATION
  FROM THE PREVIOUS CALL TO __hash_pw()!!

- If we have NIS enabled, the NIS lookup functions are called.

- The pw_breakout_yp routines see that the pw_fields bits are set and
  decline to place the data retrieved from the NIS passwd maps into the
  _pw_passwd structure.

- getpwuid() returns the results of the last __hashpw() lookup instead
  of the valid NIS data.

- Hijinxs ensue when user_from_uid() caches this bogus information and
  starts handing out the wrong usernames.

AAAARRRRRRRRRGGGGGGHHHHHHHHHH!!!

*Please* don't tell me I'm the only person to have noticed this.

Fixed by having __hashpw() check the state of pw_fields just before
bailing out on a failed lookup and clearing away any leftover garbage.
What a fun way to spend an afternoon.

1 files changed, 16 insertions, 2 deletions

diff --git a/lib/libc/gen/getpwent.c b/lib/libc/gen/getpwent.c
index e8ac1e7..e8a86a8 100644
--- a/lib/libc/gen/getpwent.c
+++ b/lib/libc/gen/getpwent.c
@@ -264,11 +264,25 @@ __hashpw(key)
 	static char *line;
 	DBT data;
 
-	if ((_pw_db->get)(_pw_db, key, &data, 0))
+	/*
+	 * XXX The pw_fields member of _pw_passwd needs to be cleared
+	 * at some point since __hashpw() can be called several times in
+	 * a single program. If we leave here after the second invokation
+	 * with garbage data in pw_fields, it can totally screw up NIS
+	 * lookups (the pw_breakout functions only populate the pw_passwd
+	 * structure if the pw_fields bits are clear).
+	 */
+	if ((_pw_db->get)(_pw_db, key, &data, 0)) {
+		if (_pw_passwd.pw_fields)
+			_pw_passwd.pw_fields = 0;
 		return(0);
+	}
 	p = (char *)data.data;
-	if (data.size > max && !(line = realloc(line, max += 1024)))
+	if (data.size > max && !(line = realloc(line, max += 1024))) {
+		if (_pw_passwd.pw_fields)
+			_pw_passwd.pw_fields = 0;
 		return(0);
+	}
 
 	t = line;
 #define	EXPAND(e)	e = t; while (*t++ = *p++);