Subject: getpwent(3) funcs return static structure
To: None <current-users@NetBSD.ORG>
From: Rick Byers <rickb@iaw.on.ca>
List: current-users
Date: 03/01/1997 19:48:47
Hi...
I found out (the hard way) that the getpwent set of functions return a
pointer to a static structure (as opposed to one that has been freshly
allocated).  Is this a good idea?  Shouldn't it atleast be mentioned in
the man pages?  This is what happened to me:

I installed CrackLib to prevent people from picking crackable passwords
(dictionary words etc...).  This required a modification of password(1).
I simple added the appropriate check in local_passwd.c.  At the top of
local_passwd(), "pw = getpwnam(uname)" is called.  At the bottom of the
function, after the necesary changes have been made to pw (changed
password), it is coppied into the password file.

The problem arose when I called "FascistCheck()" in the middle of that
function (as per the documentation for CrackLib).  FascistCheck calls
getpwuid() inside of it.  Since getpwent set of functions all operate on
the same memory area, this clobbers the original pw structure.  I tested
and installed it, thinking there wasn't any problem (I guess I didn't test
enough). 

The result is that as root: "passwd rickb", displays "changing local
password for rickb", and then proceeds to change the password for the real
userid - root!  Needless to say this caused me to have a hart attack when
the root password suddenly got changed (and continued to be changed!).

Luckily it's only the real user id that gets put in the structure (I was
afraid it was going to be the effective user id - in which case passwd
would allways change the password for root since it's setuid root!!!).

Now, the CrackLib README clearly states (near the end) 
"BUGS - it calls getpwuid(getuid()) to look up the user, this MAY affect
poorly writen programs".

I assume that passwd was not poorly written. :)  I have now modified
password to copy the structure into a new one once the function returns.

Anyway, I'll write the author of CrackLib and suggest that he may that
point more obvious, but I think the getpwent man pages should be updated
to reflect the fact that it is static.  Is there really any need for it to
be static?  I don't know a lot about this sort of thing, but it would make
more sense to me for something as critical to be either passed a pointer
to a structure to be filled, or to return a structure that was just
malloced.  Oh, now that I actually looked at the manpage, I realize that
it has to be static because subsequent calls to getpwent() read
consequtive records.  It could still be passed a pointer to a structure to
fill though, couldn't it (I guess it would be a bad idea to change it now
though, since I'm sure getpwent is fairly common).

Rick

=========================================================================
Rick Byers                                      Internet Access Worldwide
rickb@iaw.on.ca                                      System Administrator
Welland, Ontario, Canada                                    (905)714-1400
http://www.iaw.on.ca/rickb/                         http://www.iaw.on.ca/