Use an mmap()'d buffer instead of a static buffer for the contents

of the pointer returned by getpw{ent,nam,uid}().  We unmap the
buffer each time to catch callers using a stale passwd struct
pointer.  As a special case, we do not unmap the buffer if the
previous lookup was for the same name or uid.  This special case
may be removed in the future.  OK deraadt@

3 files changed, 136 insertions, 80 deletions

diff --git a/lib/libc/gen/getpwent.3 b/lib/libc/gen/getpwent.3
index c975d387544..9d842702e04 100644
--- a/lib/libc/gen/getpwent.3
+++ b/lib/libc/gen/getpwent.3
@@ -1,4 +1,4 @@
-.\"	$OpenBSD: getpwent.3,v 1.31 2016/08/14 14:57:16 tb Exp $
+.\"	$OpenBSD: getpwent.3,v 1.32 2018/08/21 20:20:04 millert Exp $
 .\"
 .\" Copyright (c) 1988, 1991, 1993
 .\"	The Regents of the University of California.  All rights reserved.
@@ -27,7 +27,7 @@
 .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 .\" SUCH DAMAGE.
 .\"
-.Dd $Mdocdate: August 14 2016 $
+.Dd $Mdocdate: August 21 2018 $
 .Dt GETPWENT 3
 .Os
 .Sh NAME
@@ -116,6 +116,15 @@ The
 .Fn getpwent
 function returns a valid pointer to a passwd structure on success
 or a null pointer if end-of-file is reached or an error occurs.
+Subsequent calls to
+.Fn getpwent ,
+.Fn getpwnam ,
+.Fn getpwnam_shadow ,
+.Fn getpwuid
+or
+.Fn getpwuid_shadow
+may invalidate the returned pointer or overwrite the contents
+of the passwd structure it points to.
 .Pp
 The
 .Fn endpwent
@@ -172,17 +181,6 @@ The historic function
 which allowed the specification of alternate password databases,
 has been deprecated and is no longer available.
 .Sh BUGS
-The
-.Fn getpwent
-function stores its results in an internal static buffer and returns
-a pointer to that buffer.
-Subsequent calls to
-.Fn getpwent ,
-.Fn getpwnam ,
-or
-.Fn getpwuid
-will overwrite the same buffer.
-.Pp
 The routines
 .Fn getpwent ,
 .Fn endpwent ,
diff --git a/lib/libc/gen/getpwent.c b/lib/libc/gen/getpwent.c
index 453e82d9169..db90e140e53 100644
--- a/lib/libc/gen/getpwent.c
+++ b/lib/libc/gen/getpwent.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: getpwent.c,v 1.61 2016/05/07 21:52:29 tedu Exp $ */
+/*	$OpenBSD: getpwent.c,v 1.62 2018/08/21 20:20:04 millert Exp $ */
 /*
  * Copyright (c) 2008 Theo de Raadt
  * Copyright (c) 1988, 1993
@@ -31,12 +31,14 @@
  */
 
 #include <sys/param.h>	/* ALIGN */
+#include <sys/mman.h>
 #include <fcntl.h>
 #include <db.h>
 #include <syslog.h>
 #include <pwd.h>
 #include <errno.h>
 #include <unistd.h>
+#include <stdbool.h>
 #include <stdlib.h>
 #include <string.h>
 #include <limits.h>
@@ -53,13 +55,21 @@
 
 #define MINIMUM(a, b)	(((a) < (b)) ? (a) : (b))
 
+struct pw_storage {
+	struct passwd pw;
+	uid_t uid;
+	char name[_PW_NAME_LEN + 1];
+	char pwbuf[_PW_BUF_LEN];
+};
+
 _THREAD_PRIVATE_KEY(pw);
 
 static DB *_pw_db;			/* password database */
 
+/* mmap'd password storage */
+static struct pw_storage *_pw_storage = MAP_FAILED;
+
 /* Following are used only by setpwent(), getpwent(), and endpwent() */
-static struct passwd _pw_passwd;	/* password structure */
-static char _pw_string[_PW_BUF_LEN];	/* string pointed to by _pw_passwd */
 static int _pw_keynum;			/* key counter */
 static int _pw_stayopen;		/* keep fd's open */
 static int _pw_flags;			/* password flags */
@@ -81,7 +91,6 @@ static char	*__ypcurrent;
 static int	__ypcurrentlen;
 static int	__yp_pw_flags;
 static struct passwd *__ypproto;
-static char	__ypline[_PW_BUF_LEN];
 static int	__getpwent_has_yppw = -1;
 static struct _ypexclude *__ypexhead;
 
@@ -251,6 +260,53 @@ __ypparse(struct passwd *pw, char *s, int yp_pw_flags)
 }
 #endif
 
+static struct passwd *
+__get_pw_buf(char **bufp, size_t *buflenp, uid_t uid, const char *name)
+{
+	bool remap = true;
+
+	/* Unmap the old buffer unless we are looking up the same uid/name */
+	if (_pw_storage != MAP_FAILED) {
+		if (name != NULL) {
+			if (strcmp(_pw_storage->name, name) == 0) {
+#ifdef PWDEBUG
+				struct syslog_data sdata = SYSLOG_DATA_INIT;
+				syslog_r(LOG_CRIT | LOG_CONS, &sdata,
+				    "repeated passwd lookup of user \"%s\"",
+				    name);
+#endif
+				remap = false;
+			}
+		} else if (uid != (uid_t)-1) {
+			if (_pw_storage->uid == uid) {
+#ifdef PWDEBUG
+				struct syslog_data sdata = SYSLOG_DATA_INIT;
+				syslog_r(LOG_CRIT | LOG_CONS, &sdata,
+				    "repeated passwd lookup of uid %u",
+				    uid);
+#endif
+				remap = false;
+			}
+		}
+		if (remap)
+			munmap(_pw_storage, sizeof(*_pw_storage));
+	}
+
+	if (remap) {
+		_pw_storage = mmap(NULL, sizeof(*_pw_storage),
+		    PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANON, -1, 0);
+		if (_pw_storage == MAP_FAILED)
+			return NULL;
+		if (name != NULL)
+			strlcpy(_pw_storage->name, name, sizeof(_pw_storage->name));
+		_pw_storage->uid = uid;
+	}
+
+	*bufp = _pw_storage->pwbuf;
+	*buflenp = sizeof(_pw_storage->pwbuf);
+	return &_pw_storage->pw;
+}
+
 struct passwd *
 getpwent(void)
 {
@@ -259,13 +315,19 @@ getpwent(void)
 	char *map;
 #endif
 	char bf[1 + sizeof(_pw_keynum)];
-	struct passwd *pw = NULL;
+	struct passwd *pw, *ret = NULL;
+	char *pwbuf;
+	size_t buflen;
 	DBT key;
 
 	_THREAD_PRIVATE_MUTEX_LOCK(pw);
 	if (!_pw_db && !__initdb(0))
 		goto done;
 
+	/* Allocate space for struct and strings, unmapping the old. */
+	if ((pw = __get_pw_buf(&pwbuf, &buflen, -1, NULL)) == NULL)
+		goto done;
+
 #ifdef YP
 	map = PASSWD_BYNAME;
 
@@ -304,13 +366,13 @@ again:
 				    &__ypcurrent, &__ypcurrentlen,
 				    &data, &datalen);
 				if (r != 0 ||
-				    __ypcurrentlen > sizeof(__ypline)) {
+				    __ypcurrentlen > buflen) {
 					__ypmode = YPMODE_NONE;
 					free(data);
 					goto again;
 				}
 			}
-			bcopy(data, __ypline, datalen);
+			bcopy(data, pwbuf, datalen);
 			free(data);
 			break;
 		case YPMODE_NETGRP:
@@ -326,7 +388,7 @@ again:
 			} else
 				goto again;
 			if (r != 0 ||
-			    __ypcurrentlen > sizeof(__ypline)) {
+			    __ypcurrentlen > buflen) {
 				/*
 				 * if the netgroup is invalid, keep looking
 				 * as there may be valid users later on.
@@ -334,7 +396,7 @@ again:
 				free(data);
 				goto again;
 			}
-			bcopy(data, __ypline, datalen);
+			bcopy(data, pwbuf, datalen);
 			free(data);
 			break;
 		case YPMODE_USER:
@@ -345,11 +407,11 @@ again:
 				free(name);
 				name = NULL;
 				if (r != 0 ||
-				    __ypcurrentlen > sizeof(__ypline)) {
+				    __ypcurrentlen > buflen) {
 					free(data);
 					goto again;
 				}
-				bcopy(data, __ypline, datalen);
+				bcopy(data, pwbuf, datalen);
 				free(data);
 			} else {		/* XXX */
 				__ypmode = YPMODE_NONE;
@@ -361,10 +423,10 @@ again:
 			break;
 		}
 
-		__ypline[datalen] = '\0';
-		if (__ypparse(&_pw_passwd, __ypline, __yp_pw_flags))
+		pwbuf[datalen] = '\0';
+		if (__ypparse(pw, pwbuf, __yp_pw_flags))
 			goto again;
-		pw = &_pw_passwd;
+		ret = pw;
 		goto done;
 	}
 #endif
@@ -374,40 +436,39 @@ again:
 	bcopy((char *)&_pw_keynum, &bf[1], sizeof(_pw_keynum));
 	key.data = (u_char *)bf;
 	key.size = 1 + sizeof(_pw_keynum);
-	if (__hashpw(&key, _pw_string, sizeof _pw_string,
-	    &_pw_passwd, &_pw_flags)) {
+	if (__hashpw(&key, pwbuf, buflen, pw, &_pw_flags)) {
 #ifdef YP
 		static long long __yppbuf[_PW_BUF_LEN / sizeof(long long)];
 		const char *user, *host, *dom;
 
 		/* if we don't have YP at all, don't bother. */
 		if (__getpwent_has_yppw) {
-			if (_pw_passwd.pw_name[0] == '+') {
+			if (pw->pw_name[0] == '+') {
 				/* set the mode */
-				switch (_pw_passwd.pw_name[1]) {
+				switch (pw->pw_name[1]) {
 				case '\0':
 					__ypmode = YPMODE_FULL;
 					break;
 				case '@':
 					__ypmode = YPMODE_NETGRP;
-					setnetgrent(_pw_passwd.pw_name + 2);
+					setnetgrent(pw->pw_name + 2);
 					break;
 				default:
 					__ypmode = YPMODE_USER;
-					name = strdup(_pw_passwd.pw_name + 1);
+					name = strdup(pw->pw_name + 1);
 					break;
 				}
 
-				__ypproto_set(&_pw_passwd, __yppbuf,
-				    _pw_flags, &__yp_pw_flags);
+				__ypproto_set(pw, __yppbuf, _pw_flags,
+				    &__yp_pw_flags);
 				goto again;
-			} else if (_pw_passwd.pw_name[0] == '-') {
+			} else if (pw->pw_name[0] == '-') {
 				/* an attempted exclusion */
-				switch (_pw_passwd.pw_name[1]) {
+				switch (pw->pw_name[1]) {
 				case '\0':
 					break;
 				case '@':
-					setnetgrent(_pw_passwd.pw_name + 2);
+					setnetgrent(pw->pw_name + 2);
 					while (getnetgrent(&host, &user, &dom)) {
 						if (user && *user)
 							__ypexclude_add(&__ypexhead,
@@ -417,20 +478,20 @@ again:
 					break;
 				default:
 					__ypexclude_add(&__ypexhead,
-					    _pw_passwd.pw_name + 1);
+					    pw->pw_name + 1);
 					break;
 				}
 				goto again;
 			}
 		}
 #endif
-		pw = &_pw_passwd;
+		ret = pw;
 		goto done;
 	}
 
 done:
 	_THREAD_PRIVATE_MUTEX_UNLOCK(pw);
-	return (pw);
+	return (ret);
 }
 
 #ifdef YP
@@ -697,10 +758,10 @@ _pwhashbyuid(uid_t uid, char *buf, size_t buflen, struct passwd *pw,
 
 static int
 getpwnam_internal(const char *name, struct passwd *pw, char *buf, size_t buflen,
-    struct passwd **pwretp, int shadow)
+    struct passwd **pwretp, bool shadow, bool reentrant)
 {
 	struct passwd *pwret = NULL;
-	int flags = 0, *flagsp;
+	int flags = 0, *flagsp = &flags;
 	int my_errno = 0;
 	int saved_errno, tmp_errno;
 
@@ -710,10 +771,12 @@ getpwnam_internal(const char *name, struct passwd *pw, char *buf, size_t buflen,
 	if (!_pw_db && !__initdb(shadow))
 		goto fail;
 
-	if (pw == &_pw_passwd)
+	if (!reentrant) {
+		/* Allocate space for struct and strings, unmapping the old. */
+		if ((pw = __get_pw_buf(&buf, &buflen, -1, name)) == NULL)
+			goto fail;
 		flagsp = &_pw_flags;
-	else
-		flagsp = &flags;
+	}
 
 #ifdef YP
 	if (__has_yppw())
@@ -743,7 +806,7 @@ int
 getpwnam_r(const char *name, struct passwd *pw, char *buf, size_t buflen,
     struct passwd **pwretp)
 {
-	return getpwnam_internal(name, pw, buf, buflen, pwretp, 0);
+	return getpwnam_internal(name, pw, buf, buflen, pwretp, false, true);
 }
 DEF_WEAK(getpwnam_r);
 
@@ -753,8 +816,7 @@ getpwnam(const char *name)
 	struct passwd *pw = NULL;
 	int my_errno;
 
-	my_errno = getpwnam_r(name, &_pw_passwd, _pw_string,
-	    sizeof _pw_string, &pw);
+	my_errno = getpwnam_internal(name, NULL, NULL, 0, &pw, false, false);
 	if (my_errno) {
 		pw = NULL;
 		errno = my_errno;
@@ -768,8 +830,7 @@ getpwnam_shadow(const char *name)
 	struct passwd *pw = NULL;
 	int my_errno;
 
-	my_errno = getpwnam_internal(name, &_pw_passwd, _pw_string,
-	    sizeof _pw_string, &pw, 1);
+	my_errno = getpwnam_internal(name, NULL, NULL, 0, &pw, true, false);
 	if (my_errno) {
 		pw = NULL;
 		errno = my_errno;
@@ -780,10 +841,10 @@ DEF_WEAK(getpwnam_shadow);
 
 static int
 getpwuid_internal(uid_t uid, struct passwd *pw, char *buf, size_t buflen,
-    struct passwd **pwretp, int shadow)
+    struct passwd **pwretp, bool shadow, bool reentrant)
 {
 	struct passwd *pwret = NULL;
-	int flags = 0, *flagsp;
+	int flags = 0, *flagsp = &flags;
 	int my_errno = 0;
 	int saved_errno, tmp_errno;
 
@@ -793,10 +854,12 @@ getpwuid_internal(uid_t uid, struct passwd *pw, char *buf, size_t buflen,
 	if (!_pw_db && !__initdb(shadow))
 		goto fail;
 
-	if (pw == &_pw_passwd)
+	if (!reentrant) {
+		/* Allocate space for struct and strings, unmapping the old. */
+		if ((pw = __get_pw_buf(&buf, &buflen, uid, NULL)) == NULL)
+			goto fail;
 		flagsp = &_pw_flags;
-	else
-		flagsp = &flags;
+	}
 
 #ifdef YP
 	if (__has_yppw())
@@ -827,7 +890,7 @@ int
 getpwuid_r(uid_t uid, struct passwd *pw, char *buf, size_t buflen,
     struct passwd **pwretp)
 {
-	return getpwuid_internal(uid, pw, buf, buflen, pwretp, 0);
+	return getpwuid_internal(uid, pw, buf, buflen, pwretp, false, true);
 }
 DEF_WEAK(getpwuid_r);
 
@@ -837,8 +900,7 @@ getpwuid(uid_t uid)
 	struct passwd *pw = NULL;
 	int my_errno;
 
-	my_errno = getpwuid_r(uid, &_pw_passwd, _pw_string,
-	    sizeof _pw_string, &pw);
+	my_errno = getpwuid_internal(uid, NULL, NULL, 0, &pw, false, false);
 	if (my_errno) {
 		pw = NULL;
 		errno = my_errno;
@@ -852,8 +914,7 @@ getpwuid_shadow(uid_t uid)
 	struct passwd *pw = NULL;
 	int my_errno;
 
-	my_errno = getpwuid_internal(uid, &_pw_passwd, _pw_string,
-	    sizeof _pw_string, &pw, 1);
+	my_errno = getpwuid_internal(uid, NULL, NULL, 0, &pw, true, false);
 	if (my_errno) {
 		pw = NULL;
 		errno = my_errno;
diff --git a/lib/libc/gen/getpwnam.3 b/lib/libc/gen/getpwnam.3
index 8d22c69058c..1936d38026d 100644
--- a/lib/libc/gen/getpwnam.3
+++ b/lib/libc/gen/getpwnam.3
@@ -1,4 +1,4 @@
-.\"	$OpenBSD: getpwnam.3,v 1.12 2017/10/17 22:47:58 schwarze Exp $
+.\"	$OpenBSD: getpwnam.3,v 1.13 2018/08/21 20:20:04 millert Exp $
 .\"
 .\" Copyright (c) 1988, 1991, 1993
 .\"	The Regents of the University of California.  All rights reserved.
@@ -27,7 +27,7 @@
 .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 .\" SUCH DAMAGE.
 .\"
-.Dd $Mdocdate: October 17 2017 $
+.Dd $Mdocdate: August 21 2018 $
 .Dt GETPWNAM 3
 .Os
 .Sh NAME
@@ -165,12 +165,22 @@ and respects the order of both normal and YP
 entries in the passwd file.
 .Sh RETURN VALUES
 The functions
-.Fn getpwnam
+.Fn getpwnam ,
+.Fn getpwnam_shadow ,
+.Fn getpwuid ,
 and
-.Fn getpwuid
-return a pointer to a passwd structure if a match is found or a
-.Dv NULL
+.Fn getpwuid_shadow
+return a pointer to a passwd structure if a match is found or a null
 pointer if no match is found or an error occurs.
+Subsequent calls to
+.Fn getpwent ,
+.Fn getpwnam ,
+.Fn getpwnam_shadow ,
+.Fn getpwuid
+or
+.Fn getpwuid_shadow
+may invalidate the returned pointer or overwrite the contents
+of the passwd structure it points to.
 .Pp
 The functions
 .Fn getpwnam_r
@@ -279,16 +289,3 @@ and
 .Fn getpwuid_shadow
 in
 .Ox 5.9 .
-.Sh BUGS
-The
-.Fn getpwnam
-and
-.Fn getpwuid
-functions store their results in an internal static buffer and return
-a pointer to that buffer.
-Subsequent calls to
-.Fn getpwent ,
-.Fn getpwnam ,
-or
-.Fn getpwuid
-will overwrite the same buffer.