[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [mituc@iasi.rdsnet.ro: pam limits drops privileges]



On Wed, 12 Sep 2001, Nalin Dahyabhai wrote:

> On Wed, Sep 12, 2001 at 02:32:27PM -0500, Steve Langasek wrote:
> > Is the following ok for a first implementation, or do you have more ambitious
> > plans?  I believe you mentioned providing a wrapper for systems that don't
> > have getpwnam_r(), but I'm personally quite content with this.
> [snip]
> > #if HAVE_GETPWNAM_R
> >     getpwnam_r(name, pwd, buf, sizeof(buf), &pwd);
> > #else
> >     pwd = getpwnam(name);
> > #endif

> I tried this route, and discovered that if getpwnam_r fails, you have
> no guarantees about the value of the parameters you passed in (pwd
> may be unchanged, or it may point to the struct passwd, or to some
> other location), so it's probably better to do this:

Hmm.  I've never used any of these functions before, so I know only what's
found in libc.info.  There, it's claimed that the pointer given as the last
argument will be set to NULL if the call fails, but if that doesn't stand up
to empirical testing, then I'll keep that in mind.


> 	if(getpwnam_r(name, pwd, buf, sizeof(buf), &pwd) != 0) {
> 		pwd = NULL;
> 	}

> But Thorsten's correct that this doesn't catch ERANGE problems which
> are likely to crop up when you have really big groups.  I was toying
> with the attached piece of untested code (it's a bit more complicated
> than it might need to be, because I'm trying to elimitate duplicate
> code paths in the main querying function, and it allocates a separate
> PAM data item for use with each query).  Any comments about it?

Seems to trade reentrancy problems in the system as a whole for reentrancy
problems within the modules.  I'd rather that each function/module that's a
consumer of getpwnam_r() be expected to keep track of its own buffers and free
them when necessary.

If any wrappers are to be used, it should be to provide a getpwnam_r()
work-alike for systems that lack this function call.  This would minimize (but
of course, not eliminate) race conditions surrounding the use of these static
buffers.  Insert a semaphore locking mechanism too, if you like.

Code included inline below.

Steve Langasek
postmodern programmer


#ifndef HAVE_GETPWNAM_R
int getpwnam_r ((const char *name, struct passwd *resultbuf,
                 char *buffer, size_t buflen, struct passwd **result))
{

	struct passwd *pwd;
	ptrdiff_t i = 0;
	/* Avoid buffer overflows if another thread calls getpwnam() after
	   we've verified the string lengths */
	size_t namelen, pwdlen, gecoslen, dirlen, shelllen;

	pwd = getpwnam(name);

	if (!pwd) {
		*result = NULL;
		if (errno != 0) {
			return errno;
		}
		return 0;
	}

	namelen = strlen(pwd->pw_name) + 1;
	pwdlen = strlen(pwd->pw_passwd) + 1;
	gecoslen = strlen(pwd->pw_gecos) + 1;
	dirlen = strlen(pwd->pw_dir) + 1;
	shelllen = strlen(pwd->pw_shell) + 1;
	if (namelen + pwdlen + gecoslen + dirlen + shelllen > buflen)
	{
		*result = NULL;
		errno = ERANGE;
		return ERANGE;
	}

	strncpy(buffer, pwd->pw_name, namelen);
	resultbuf->pw_name = buffer;
	buffer += namelen;
	*(buffer - 1) = '\0';

	strncpy(buffer, pwd->pw_passwd, pwdlen);
	resultbuf->pw_passwd = buffer;
	buffer += pwdlen;
	*(buffer - 1) = '\0';

	strncpy(buffer, pwd->pw_gecos, gecoslen);
	resultbuf->pw_gecos = buffer;
	buffer += gecoslen;
	*(buffer - 1) = '\0';

	strncpy(buffer, pwd->pw_dir, dirlen);
	resultbuf->pw_dir = buffer;
	buffer += dirlen;
	*(buffer - 1) = '\0';

	strncpy(buffer, pwd->pw_shell, shelllen);
	resultbuf->pw_shell = buffer;
	buffer += dirlen;
	*(buffer - 1) = '\0';

	*result = resultbuf;
	return 0;
}
#endif





[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index] []