From: millert <millert@openbsd.org>
Date: Fri, 21 Sep 2018 19:00:45 +0000 (+0000)
Subject: Use password/group cache functions and avoid stashing a pointer to
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=78f0238b1164c3b9a0fe418e53d4baae114fe7f7;p=openbsd

Use password/group cache functions and avoid stashing a pointer to
the return value of getgrgid(3) or getgrnam(3) which relies on
undefined behavior.  The rdist server will now use getgroups(2) to
determine group membership of the invoking user.  In addition, there
is now one implementation of tilde expansion instead of two.
OK tb@ tim@
---

diff --git a/usr.bin/rdist/common.c b/usr.bin/rdist/common.c
index 62efc8806a1..7b7096aaacc 100644
--- a/usr.bin/rdist/common.c
+++ b/usr.bin/rdist/common.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: common.c,v 1.38 2018/09/09 13:53:11 millert Exp $	*/
+/*	$OpenBSD: common.c,v 1.39 2018/09/21 19:00:45 millert Exp $	*/
 
 /*
  * Copyright (c) 1983 Regents of the University of California.
@@ -56,6 +56,8 @@
 char			host[HOST_NAME_MAX+1];	/* Name of this host */
 uid_t			userid = (uid_t)-1;	/* User's UID */
 gid_t			groupid = (gid_t)-1;	/* User's GID */
+gid_t		        gidset[NGROUPS_MAX];	/* User's GID list */
+int			gidsetlen = 0;		/* Number of GIDS in list */
 char		       *homedir = NULL;		/* User's $HOME */
 char		       *locuser = NULL;		/* Local User's name */
 int			isserver = FALSE;	/* We're the server */
@@ -129,6 +131,7 @@ init(int argc, char **argv, char **envp)
 	homedir = xstrdup(pw->pw_dir);
 	locuser = xstrdup(pw->pw_name);
 	groupid = pw->pw_gid;
+	gidsetlen = getgroups(NGROUPS_MAX, gidset);
 	gethostname(host, sizeof(host));
 #if 0
 	if ((cp = strchr(host, '.')) != NULL)
@@ -436,7 +439,7 @@ getusername(uid_t uid, char *file, opt_t opts)
 {
 	static char buf[100];
 	static uid_t lastuid = (uid_t)-1;
-	struct passwd *pwd = NULL;
+	const char *name;
 
 	/*
 	 * The value of opts may have changed so we always
@@ -448,14 +451,14 @@ getusername(uid_t uid, char *file, opt_t opts)
   	}
 
 	/*
-	 * Try to avoid getpwuid() call.
+	 * Try to avoid passwd lookup.
 	 */
 	if (lastuid == uid && buf[0] != '\0' && buf[0] != ':')
 		return(buf);
 
 	lastuid = uid;
 
-	if ((pwd = getpwuid(uid)) == NULL) {
+	if ((name = user_from_uid(uid, 1)) == NULL) {
 		if (IS_ON(opts, DO_DEFOWNER) && !isserver) 
 			(void) strlcpy(buf, defowner, sizeof(buf));
 		else {
@@ -464,7 +467,7 @@ getusername(uid_t uid, char *file, opt_t opts)
 			(void) snprintf(buf, sizeof(buf), ":%u", uid);
 		}
 	} else {
-		(void) strlcpy(buf, pwd->pw_name, sizeof(buf));
+		(void) strlcpy(buf, name, sizeof(buf));
 	}
 
 	return(buf);
@@ -478,7 +481,7 @@ getgroupname(gid_t gid, char *file, opt_t opts)
 {
 	static char buf[100];
 	static gid_t lastgid = (gid_t)-1;
-	struct group *grp = NULL;
+	const char *name;
 
 	/*
 	 * The value of opts may have changed so we always
@@ -490,14 +493,14 @@ getgroupname(gid_t gid, char *file, opt_t opts)
   	}
 
 	/*
-	 * Try to avoid getgrgid() call.
+	 * Try to avoid group lookup.
 	 */
 	if (lastgid == gid && buf[0] != '\0' && buf[0] != ':')
 		return(buf);
 
 	lastgid = gid;
 
-	if ((grp = (struct group *)getgrgid(gid)) == NULL) {
+	if ((name = group_from_gid(gid, 1)) == NULL) {
 		if (IS_ON(opts, DO_DEFGROUP) && !isserver) 
 			(void) strlcpy(buf, defgroup, sizeof(buf));
 		else {
@@ -506,7 +509,7 @@ getgroupname(gid_t gid, char *file, opt_t opts)
 			(void) snprintf(buf, sizeof(buf), ":%u", gid);
 		}
 	} else
-		(void) strlcpy(buf, grp->gr_name, sizeof(buf));
+		(void) strlcpy(buf, name, sizeof(buf));
 
 	return(buf);
 }
@@ -574,6 +577,8 @@ exptilde(char *ebuf, char *file, size_t ebufsize)
 {
 	struct passwd *pw;
 	char *pw_dir, *rest;
+	static char lastuser[_PW_NAME_LEN + 1];
+	static char lastdir[PATH_MAX];
 	size_t len;
 
 	if (*file != '~') {
@@ -595,13 +600,17 @@ notilde:
 		else
 			rest = NULL;
 		if (strcmp(locuser, file) != 0) {
-			if ((pw = getpwnam(file)) == NULL) {
-				error("%s: unknown user name", file);
-				if (rest != NULL)
-					*rest = '/';
-				return(NULL);
+			if (strcmp(lastuser, file) != 0) {
+				if ((pw = getpwnam(file)) == NULL) {
+					error("%s: unknown user name", file);
+					if (rest != NULL)
+						*rest = '/';
+					return(NULL);
+				}
+				strlcpy(lastuser, pw->pw_name, sizeof(lastuser));
+				strlcpy(lastdir, pw->pw_dir, sizeof(lastdir));
 			}
-			pw_dir = pw->pw_dir;
+			pw_dir = lastdir;
 		}
 		if (rest != NULL)
 			*rest = '/';
diff --git a/usr.bin/rdist/defs.h b/usr.bin/rdist/defs.h
index 88ae0778649..e63ee3d801c 100644
--- a/usr.bin/rdist/defs.h
+++ b/usr.bin/rdist/defs.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: defs.h,v 1.37 2018/09/09 13:53:11 millert Exp $	*/
+/*	$OpenBSD: defs.h,v 1.38 2018/09/21 19:00:45 millert Exp $	*/
 
 #ifndef __DEFS_H__
 #define __DEFS_H__
@@ -167,6 +167,8 @@ extern int		rem_w;		/* Remote file descriptor, writing */
 extern int		rtimeout;	/* Response time out in seconds */
 extern uid_t		userid;		/* User ID of rdist user */
 extern gid_t		groupid;	/* Group ID of rdist user */
+extern gid_t	        gidset[];	/* List of group IDs of rdist user */
+extern int		gidsetlen;	/* Number of group IDs in list */
 extern jmp_buf		finish_jmpbuf;	/* Setjmp buffer for finish() */
 extern char defowner[64];		/* Default owner */
 extern char defgroup[64];		/* Default group */
diff --git a/usr.bin/rdist/expand.c b/usr.bin/rdist/expand.c
index bdc158281a1..bc9209db67b 100644
--- a/usr.bin/rdist/expand.c
+++ b/usr.bin/rdist/expand.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: expand.c,v 1.16 2018/09/09 13:53:11 millert Exp $	*/
+/*	$OpenBSD: expand.c,v 1.17 2018/09/21 19:00:45 millert Exp $	*/
 
 /*
  * Copyright (c) 1983 Regents of the University of California.
@@ -52,6 +52,7 @@ char	       *pathp;
 char	       *lastpathp;
 char	       *tilde;		/* "~user" if not expanding tilde, else "" */
 char	       *tpathp;
+char		pathbuf[BUFSIZ];
 
 int		expany;		/* any expansions done? */
 char	       *entp;
@@ -116,7 +117,6 @@ expand(struct namelist *list, int wh)		/* quote in list->n_name */
 {
 	struct namelist *nl, *prev;
 	int n;
-	char pathbuf[BUFSIZ];
 
 	if (debug)
 		debugmsg(DM_CALL, "expand(%p, %d) start, list = %s", 
@@ -260,36 +260,16 @@ expstr(u_char *s)
 		return;
 	}
 	if (*s == '~') {
-		struct passwd *pw;
-
-		cp = ++s;
-		if (*cp == CNULL || *cp == '/') {
+		if ((cp = strchr(s, '/')) == NULL) {
 			tilde = "~";
-			cp1 = (u_char *)homedir;
+			s++;
 		} else {
-			tilde = (char *)(cp1 = ebuf);
-			*cp1++ = '~';
-			do
-				*cp1++ = *cp++;
-			while (*cp && *cp != '/');
-			*cp1 = CNULL;
-			if (strcmp(locuser, (char *)ebuf+1) != 0) {
-				if ((pw = getpwnam((char *)ebuf+1)) == NULL) {
-					strlcat((char *)ebuf, 
-					        ": unknown user name",
-					        sizeof(ebuf));
-					yyerror((char *)ebuf+1);
-					return;
-				}
-				cp1 = (u_char *)pw->pw_dir;
-			} else {
-				cp1 = (u_char *)homedir;
-			}
+			tilde = memcpy(ebuf, s, (cp - s));
+			ebuf[cp - s] = '\0';
 			s = cp;
 		}
-		for (cp = (u_char *)path; (*cp++ = *cp1++) != '\0'; )
-			continue;
-		tpathp = pathp = (char *)cp - 1;
+		cp = exptilde(path, tilde, sizeof(pathbuf));
+		tpathp = pathp = (char *)cp;
 	} else {
 		tpathp = pathp = path;
 		tilde = "";