fix double shell expansion issue found in scp in rcp too; ok deraadt

3 files changed, 186 insertions, 61 deletions

diff --git a/bin/rcp/extern.h b/bin/rcp/extern.h
index eafb8b630d2..69e6c2aa769 100644
--- a/bin/rcp/extern.h
+++ b/bin/rcp/extern.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: extern.h,v 1.5 2003/06/02 23:32:09 millert Exp $	*/
+/*	$OpenBSD: extern.h,v 1.6 2006/03/12 01:51:15 djm Exp $	*/
 /*	$NetBSD: extern.h,v 1.2 1995/03/21 08:19:01 cgd Exp $	*/
 
 /*-
@@ -40,11 +40,23 @@ typedef struct {
 extern int iamremote;
 extern char *__progname;
 
+typedef struct arglist arglist;
+struct arglist {
+	char    **list;
+	u_int   num;
+	u_int   nalloc;
+};
+void	 addargs(arglist *, char *, ...)
+	     __attribute__((format(printf, 2, 3)));
+void	 replacearg(arglist *, u_int, char *, ...)
+	     __attribute__((format(printf, 3, 4)));
+void	 freeargs(arglist *);
+
 BUF	*allocbuf(BUF *, int, int);
 char	*colon(char *);
 void	 lostconn(int);
 void	 nospace(void);
 int	 okname(char *);
 void	 run_err(const char *, ...);
-int	 susystem(char *, int);
 void	 verifydir(char *);
+int	 do_local_cmd(arglist *, uid_t, gid_t);
diff --git a/bin/rcp/rcp.c b/bin/rcp/rcp.c
index b69f178880c..f5fe5a1f027 100644
--- a/bin/rcp/rcp.c
+++ b/bin/rcp/rcp.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rcp.c,v 1.42 2005/11/12 18:34:25 deraadt Exp $	*/
+/*	$OpenBSD: rcp.c,v 1.43 2006/03/12 01:51:15 djm Exp $	*/
 /*	$NetBSD: rcp.c,v 1.9 1995/03/21 08:19:06 cgd Exp $	*/
 
 /*
@@ -40,7 +40,7 @@ static char copyright[] =
 #if 0
 static char sccsid[] = "@(#)rcp.c	8.2 (Berkeley) 4/2/94";
 #else
-static const char rcsid[] = "$OpenBSD: rcp.c,v 1.42 2005/11/12 18:34:25 deraadt Exp $";
+static const char rcsid[] = "$OpenBSD: rcp.c,v 1.43 2006/03/12 01:51:15 djm Exp $";
 #endif
 #endif /* not lint */
 
@@ -87,6 +87,7 @@ int	doencrypt = 0;
 struct passwd *pwd;
 u_short	port;
 uid_t	userid;
+gid_t	groupid;
 int errs, rem;
 int pflag, iamremote, iamrecursive, targetshouldbedirectory;
 
@@ -177,6 +178,7 @@ main(int argc, char *argv[])
 
 	if ((pwd = getpwuid(userid = getuid())) == NULL)
 		errx(1, "unknown user %u", userid);
+	groupid = pwd->pw_gid;
 
 	unsetenv("RSH");		/* Force the use of /usr/bin/rsh */
 
@@ -184,15 +186,17 @@ main(int argc, char *argv[])
 
 	if (fflag) {			/* Follow "protocol", send data. */
 		(void)response();
-		(void)seteuid(userid);
-		(void)setuid(userid);
+		(void)setresgid(groupid, groupid, groupid);
+		(void)setgroups(1, &groupid);
+		(void)setresuid(userid, userid, userid);
 		source(argc, argv);
 		exit(errs != 0);
 	}
 
 	if (tflag) {			/* Receive data. */
-		(void)seteuid(userid);
-		(void)setuid(userid);
+		(void)setresgid(groupid, groupid, groupid);
+		(void)setgroups(1, &groupid);
+		(void)setresuid(userid, userid, userid);
 		sink(argc, argv);
 		exit(errs != 0);
 	}
@@ -232,6 +236,10 @@ toremote(char *targ, int argc, char *argv[])
 {
 	int i, tos;
 	char *bp, *host, *src, *suser, *thost, *tuser, *user, *arg;
+	arglist alist;
+
+	memset(&alist, '\0', sizeof(alist));
+	alist.list = NULL;
 
 	if ((user = strdup(pwd->pw_name)) == NULL)
 		err(1, "malloc");
@@ -259,9 +267,14 @@ toremote(char *targ, int argc, char *argv[])
 	for (i = 0; i < argc - 1; i++) {
 		src = colon(argv[i]);
 		if (src) {			/* remote to remote */
+			freeargs(&alist);
+			addargs(&alist, "%s", _PATH_RSH);
+			addargs(&alist, "%s", "-n");
+
 			*src++ = 0;
 			if (*src == 0)
 				src = ".";
+
 			host = strchr(argv[i], '@');
 			if (host) {
 				*host++ = 0;
@@ -270,22 +283,18 @@ toremote(char *targ, int argc, char *argv[])
 					suser = user;
 				else if (!okname(suser))
 					continue;
-				if (asprintf(&bp,
-				    "%s %s -l %s -n %s %s '%s%s%s:%s'",
-				    _PATH_RSH, host, suser, cmd, src,
-				    tuser ? tuser : "", tuser ? "@" : "",
-				    thost, targ) == -1)
-					err(1, NULL);
-			} else {
-				if (asprintf(&bp,
-				    "exec %s %s -n %s %s '%s%s%s:%s'",
-				    _PATH_RSH, argv[i], cmd, src,
-				    tuser ? tuser : "", tuser ? "@" : "",
-				    thost, targ) == -1)
-					err(1, NULL);
-			}
-			(void)susystem(bp, userid);
-			(void)free(bp);
+
+				addargs(&alist, "-l");
+				addargs(&alist, "%s", suser);
+			} else
+				host = argv[1];
+			addargs(&alist, "%s", host);
+			addargs(&alist, "%s", cmd);
+			addargs(&alist, "%s", src);
+			addargs(&alist, "%s%s%s:%s",
+			    tuser ? tuser : "", tuser ? "@" : "",
+			    thost, targ);
+			do_local_cmd(&alist, userid, groupid);
 		} else {			/* local to remote */
 			if (rem == -1) {
 				if (asprintf(&bp, "%s -t %s", cmd, targ) == -1)
@@ -309,8 +318,9 @@ toremote(char *targ, int argc, char *argv[])
 				if (response() < 0)
 					exit(1);
 				(void)free(bp);
-				(void)seteuid(userid);
-				(void)setuid(userid);
+				(void)setresgid(groupid, groupid, groupid);
+				(void)setgroups(1, &groupid);
+				(void)setresuid(userid, userid, userid);
 			}
 			source(1, argv+i);
 		}
@@ -323,19 +333,26 @@ tolocal(int argc, char *argv[])
 {
 	int i, tos;
 	char *bp, *host, *src, *suser, *user;
+	arglist alist;
+
+	memset(&alist, '\0', sizeof(alist));
+	alist.list = NULL;
 
 	if ((user = strdup(pwd->pw_name)) == NULL)
 		err(1, "malloc");
 
 	for (i = 0; i < argc - 1; i++) {
 		if (!(src = colon(argv[i]))) {		/* Local to local. */
-			if (asprintf(&bp, "exec %s%s%s %s %s", _PATH_CP,
-			    iamrecursive ? " -R" : "", pflag ? " -p" : "",
-			    argv[i], argv[argc - 1]) == -1)
-				err(1, NULL);
-			if (susystem(bp, userid))
+			freeargs(&alist);
+			addargs(&alist, "%s", _PATH_CP);
+			if (iamrecursive)
+				addargs(&alist, "-R");
+			if (pflag)
+				addargs(&alist, "-p");
+			addargs(&alist, "%s", argv[i]);
+			addargs(&alist, "%s", argv[argc-1]);
+			if (do_local_cmd(&alist, userid, groupid))
 				++errs;
-			(void)free(bp);
 			continue;
 		}
 		*src++ = 0;
diff --git a/bin/rcp/util.c b/bin/rcp/util.c
index 8686b09d4c5..308dc95ff6d 100644
--- a/bin/rcp/util.c
+++ b/bin/rcp/util.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: util.c,v 1.15 2004/09/14 22:06:19 deraadt Exp $	*/
+/*	$OpenBSD: util.c,v 1.16 2006/03/12 01:51:15 djm Exp $	*/
 /*	$NetBSD: util.c,v 1.2 1995/03/21 08:19:08 cgd Exp $	*/
 
 /*-
@@ -34,7 +34,7 @@
 #if 0
 static char sccsid[] = "@(#)util.c	8.2 (Berkeley) 4/2/94";
 #else
-static const char rcsid[] = "$OpenBSD: util.c,v 1.15 2004/09/14 22:06:19 deraadt Exp $";
+static const char rcsid[] = "$OpenBSD: util.c,v 1.16 2006/03/12 01:51:15 djm Exp $";
 #endif
 #endif /* not lint */
 
@@ -51,9 +51,12 @@ static const char rcsid[] = "$OpenBSD: util.c,v 1.15 2004/09/14 22:06:19 deraadt
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <stdarg.h>
 
 #include "extern.h"
 
+static pid_t do_cmd_pid = -1; /* PID of subprocess during do_local_cmd() */
+
 char *
 colon(char *cp)
 {
@@ -100,33 +103,6 @@ bad:	warnx("%s: invalid user name", cp0);
 	return (0);
 }
 
-int
-susystem(char *s, int userid)
-{
-	sig_t istat, qstat;
-	int status;
-	pid_t pid;
-
-	pid = vfork();
-	switch (pid) {
-	case -1:
-		return (127);
-
-	case 0:
-		(void)seteuid(userid);
-		(void)setuid(userid);
-		execl(_PATH_BSHELL, "sh", "-c", s, (char *)NULL);
-		_exit(127);
-	}
-	istat = signal(SIGINT, SIG_IGN);
-	qstat = signal(SIGQUIT, SIG_IGN);
-	if (waitpid(pid, &status, 0) < 0)
-		status = -1;
-	(void)signal(SIGINT, istat);
-	(void)signal(SIGQUIT, qstat);
-	return (status);
-}
-
 BUF *
 allocbuf(BUF *bp, int fd, int blksize)
 {
@@ -170,3 +146,123 @@ lostconn(int signo)
 	}
 	_exit(1);
 }
+
+
+static void
+killchild(int signo)
+{
+	if (do_cmd_pid > 1) {
+		kill(do_cmd_pid, signo ? signo : SIGTERM);
+		waitpid(do_cmd_pid, NULL, 0);
+	}
+
+	if (signo)
+		_exit(1);
+	exit(1);
+}
+
+int
+do_local_cmd(arglist *a, uid_t userid, gid_t groupid)
+{
+	u_int i;
+	int status;
+	pid_t pid;
+
+	if (a->num == 0)
+		errx(1, "do_local_cmd: no arguments");
+
+	if ((pid = fork()) == -1)
+		err(1, "do_local_cmd: fork");
+
+	if (pid == 0) {
+		setresgid(groupid, groupid, groupid);
+		setgroups(1, &groupid);
+		setresuid(userid, userid, userid);
+		execvp(a->list[0], a->list);
+		perror(a->list[0]);
+		exit(1);
+	}
+
+	do_cmd_pid = pid;
+	signal(SIGTERM, killchild);
+	signal(SIGINT, killchild);
+	signal(SIGHUP, killchild);
+
+	while (waitpid(pid, &status, 0) == -1)
+		if (errno != EINTR)
+			err(1, "do_local_cmd: waitpid");
+
+	do_cmd_pid = -1;
+
+	if (!WIFEXITED(status) || WEXITSTATUS(status) != 0)
+		return (-1);
+
+	signal(SIGTERM, SIG_DFL);
+	signal(SIGINT, SIG_DFL);
+	signal(SIGHUP, SIG_DFL);
+
+	return (0);
+}
+
+/* function to assist building execv() arguments */
+void
+addargs(arglist *args, char *fmt, ...)
+{
+	va_list ap;
+	char *cp;
+	u_int nalloc;
+	int r;
+
+	va_start(ap, fmt);
+	r = vasprintf(&cp, fmt, ap);
+	va_end(ap);
+	if (r == -1)
+		errx(1, "addargs: argument too long");
+
+	nalloc = args->nalloc;
+	if (args->list == NULL) {
+		nalloc = 32;
+		args->num = 0;
+	} else if (args->num+2 >= nalloc)
+		nalloc *= 2;
+
+	if ((args->list = realloc(args->list, nalloc * sizeof(char *))) == NULL)
+		errx(1, "addargs: realloc failed");
+	args->nalloc = nalloc;
+	args->list[args->num++] = cp;
+	args->list[args->num] = NULL;
+}
+
+void
+replacearg(arglist *args, u_int which, char *fmt, ...)
+{
+	va_list ap;
+	char *cp;
+	int r;
+
+	va_start(ap, fmt);
+	r = vasprintf(&cp, fmt, ap);
+	va_end(ap);
+	if (r == -1)
+		errx(1, "replacearg: argument too long");
+
+	if (which >= args->num)
+		errx(1, "replacearg: tried to replace invalid arg %d >= %d",
+		    which, args->num);
+	free(args->list[which]);
+	args->list[which] = cp;
+}
+
+void
+freeargs(arglist *args)
+{
+	u_int i;
+
+	if (args->list != NULL) {
+		for (i = 0; i < args->num; i++)
+			free(args->list[i]);
+		free(args->list);
+		args->nalloc = args->num = 0;
+		args->list = NULL;
+	}
+}