Removed the transfer of most environment variables, such as USER, HOME,

and PATH from the original user to the target user. This could cause
files in the wrogn path or home directory to be read (or written to),
which resulted in potential security problems.

This has been changed so that only DISPLAY and TERM are passed to the
new environment. This is fine for running command line programs. When
GUI programs need to be run, "keepenv" can be added to the user's
doas.conf entry. This results in variables like HOME being copied
to the target user, allowing GUI programs to run.

Many thanks to Sander Bos for reporting this issue and explaining
how it can be exploited.

This commit also adds the ability to pass a customized PATH to
target users. The new PATH can be set at compile time in the
Makefile. The default path is provided in the Makefile and commented
out.

Makefile

@@ -3,7 +3,9 @@ YACC?=yacc
 BIN=doas
 PREFIX?=/usr/local
 OBJECTS=doas.o env.o execvpe.o reallocarray.o y.tab.o
-CFLAGS+=-DUSE_PAM -DDOAS_CONF=\"${PREFIX}/etc/doas.conf\" 
+# Can set GLOBAL_PATH here to set PATH for target user.
+# TARGETPATH=-DGLOBAL_PATH=\"/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:\"
+CFLAGS+=-DUSE_PAM -DDOAS_CONF=\"${PREFIX}/etc/doas.conf\" $(TARGETPATH) 
 LDFLAGS+=-lpam
 UNAME_S := $(shell uname -s)
 ifeq ($(UNAME_S),Linux)
@@ -31,7 +33,7 @@ y.tab.o: parse.y
 	$(YACC) parse.y
 	$(CC) $(CFLAGS) -c y.tab.c
 
-install: all
+install: $(BIN)
 	mkdir -p $(PREFIX)/bin
 	cp $(BIN) $(PREFIX)/bin/
 	chmod 4755 $(PREFIX)/bin/$(BIN)

doas.c

@@ -313,7 +313,7 @@ main(int argc, char **argv)
 	const char *cmd;
 	char cmdline[LINE_MAX];
 	char myname[_PW_NAME_LEN + 1];
-	struct passwd *pw;
+	struct passwd *original_pw, *target_pw;
 	struct rule *rule;
 	uid_t uid;
 	uid_t target = 0;
@@ -376,10 +376,10 @@ main(int argc, char **argv)
 	} else if ((!sflag && !argc) || (sflag && argc))
 		usage();
 
-	pw = getpwuid(uid);
+	original_pw = getpwuid(uid);
-	if (!pw)
+	if (! original_pw)
 		err(1, "getpwuid failed");
-	if (strlcpy(myname, pw->pw_name, sizeof(myname)) >= sizeof(myname))
+	if (strlcpy(myname, original_pw->pw_name, sizeof(myname)) >= sizeof(myname))
 		errx(1, "pw_name too long");
 
 	ngroups = getgroups(NGROUPS_MAX, groups);
@@ -390,7 +390,7 @@ main(int argc, char **argv)
 	if (sflag) {
 		sh = getenv("SHELL");
 		if (sh == NULL || *sh == '\0') {
-			shargv[0] = strdup(pw->pw_shell);
+			shargv[0] = strdup(original_pw->pw_shell);
 			if (shargv[0] == NULL)
 				err(1, NULL);
 		} else
@@ -540,12 +540,12 @@ main(int argc, char **argv)
 	if (pledge("stdio rpath getpw exec id", NULL) == -1)
 		err(1, "pledge");
         */
-	pw = getpwuid(target);
+	target_pw = getpwuid(target);
-	if (!pw)
+	if (! target_pw)
 		errx(1, "no passwd entry for target");
 
 #if defined(HAVE_LOGIN_CAP_H)
-	if (setusercontext(NULL, pw, target, LOGIN_SETGROUP |
+	if (setusercontext(NULL, target_pw, target, LOGIN_SETGROUP |
 	    LOGIN_SETPRIORITY | LOGIN_SETRESOURCES | LOGIN_SETUMASK |
 	    LOGIN_SETUSER) != 0)
 		errx(1, "failed to set user context for target");
@@ -574,9 +574,9 @@ main(int argc, char **argv)
 #endif
 
 	syslog(LOG_AUTHPRIV | LOG_INFO, "%s ran command %s as %s from %s",
-	    myname, cmdline, pw->pw_name, cwd);
+	    myname, cmdline, target_pw->pw_name, cwd);
 
-	envp = prepenv(rule);
+	envp = prepenv(rule, original_pw, target_pw);
 
 	if (rule->cmd) {
 		if (setenv("PATH", safepath, 1) == -1)

doas.conf.5

@@ -53,15 +53,20 @@ again for some time. Works on OpenBSD only, persist is not available on Linux or
 .It Ic keepenv
 The user's environment is maintained.
 The default is to reset the environment, except for the variables
-.Ev DISPLAY ,
+.Ev DISPLAY 
-.Ev HOME ,
-.Ev LOGNAME ,
-.Ev MAIL ,
-.Ev PATH ,
-.Ev TERM ,
-.Ev USER
 and
-.Ev USERNAME .
+.Ev TERM .
+
+Note: In order to be able to run most desktop (GUI) applications, the user needs to
+have the keepenv keyword specified. If keepenv is not specified then key elements, like
+the user's $HOME variable, will be reset and cause the GUI application to crash.
+Users who only need to run command line applications can usually get away without
+keepenv. When in doubt, try to avoid using keepenv as it is less secure to have
+environment variables passed to privileged users.
+
+Note: The target user's PATH variable can be set at compile time by adjusting the
+GLOBAL_PATH variable in doas's Makefile. By default, the target user's path will
+be set to "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:"
 .It Ic setenv { Oo Ar variable ... Oc Oo Ar variable=value ... Oc Ic }
 In addition to the variables mentioned above, keep the space-separated
 specified variables.

doas.h

@@ -29,7 +29,12 @@ extern struct rule **rules;
 extern int nrules;
 extern int parse_errors;
 
-char **prepenv(struct rule *);
+struct passwd;
+char **prepenv(struct rule *, struct passwd *original, struct passwd *target);
+
+#ifndef GLOBAL_PATH 
+#define GLOBAL_PATH "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
+#endif
 
 #define PERMIT	1
 #define DENY	2

env.c

@@ -23,6 +23,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <pwd.h>
 #include <err.h>
 #include <unistd.h>
 #include <errno.h>
@@ -77,8 +78,19 @@ freenode(struct envnode *node)
 	free(node);
 }
 
+static void
+addnode(struct env *env, const char *key, const char *value)
+{
+	struct envnode *node;
+
+	node = createnode(key, value);
+	RB_INSERT(envtree, &env->root, node);
+	env->count++;
+}
+
+
 static struct env *
-createenv(struct rule *rule)
+createenv(struct rule *rule, struct passwd *original, struct passwd *target)
 {
 	struct env *env;
 	u_int i;
@@ -89,6 +101,13 @@ createenv(struct rule *rule)
 	RB_INIT(&env->root);
 	env->count = 0;
 
+        addnode(env, "DOAS_USER", original->pw_name);
+	addnode(env, "HOME", target->pw_dir);
+	addnode(env, "LOGNAME", target->pw_name);
+	addnode(env, "PATH", GLOBAL_PATH); 
+	addnode(env, "SHELL", target->pw_shell);
+	addnode(env, "USER", target->pw_name);
+
 	if (rule->options & KEEPENV) {
                 #ifndef linux
 		extern const char **environ;
@@ -197,16 +216,15 @@ fillenv(struct env *env, const char **envlist)
 }
 
 char **
-prepenv(struct rule *rule)
+prepenv(struct rule *rule, struct passwd *original, struct passwd *target)
 {
-	static const char *safeset[] = {
+        static const char *safeset[] = {
-		"DISPLAY", "HOME", "LOGNAME", "MAIL",
+                "DISPLAY", "TERM", NULL
-		"PATH", "TERM", "USER", "USERNAME",
+        };
-		NULL
+
-	};
 	struct env *env;
 
-	env = createenv(rule);
+	env = createenv(rule, original, target);
 
 	/* if we started with blank, fill some defaults then apply rules */
 	if (!(rule->options & KEEPENV))