From 01c658f8c45cb92a343be5f32aa6da70b2032168 Mon Sep 17 00:00:00 2001
From: tedu <tedu>
Date: Sun, 16 Jun 2019 18:16:34 +0000
Subject: [PATCH] redo the environment inheritance to not inherit. it was
 intended to make life easier, but it can be surprising or even unsafe.
 instead, reset just about everything to the target user's values. ok deraadt
 martijn Thanks to Sander Bos in particular for pointing out some nasty edge
 cases.

---
 doas.c      |  4 +++-
 doas.conf.5 | 10 ++--------
 doas.h      |  5 ++++-
 env.c       | 45 +++++++++++++++++++++++++++++++++------------
 4 files changed, 42 insertions(+), 22 deletions(-)

diff --git a/doas.c b/doas.c
index 1fd0e9a..5396df0 100644
--- a/doas.c
+++ b/doas.c
@@ -449,6 +449,7 @@ main(int argc, char **argv)
 
 #ifdef HAVE_SETUSERCONTEXT
 	if (setusercontext(NULL, targpw, target, LOGIN_SETGROUP |
+	    LOGIN_SETPATH |
 	    LOGIN_SETPRIORITY | LOGIN_SETRESOURCES | LOGIN_SETUMASK |
 	    LOGIN_SETUSER) != 0)
 		errx(1, "failed to set user context for target");
@@ -479,9 +480,10 @@ main(int argc, char **argv)
 	syslog(LOG_AUTHPRIV | LOG_INFO, "%s ran command %s as %s from %s",
 	    mypw->pw_name, cmdline, targpw->pw_name, cwd);
 
-	envp = prepenv(rule);
+	envp = prepenv(rule, mypw, targpw);
 
 	if (rule->cmd) {
+		/* do this again after setusercontext reset it */
 		if (setenv("PATH", safepath, 1) == -1)
 			err(1, "failed to set PATH '%s'", safepath);
 	}
diff --git a/doas.conf.5 b/doas.conf.5
index 95daf4c..8fd700b 100644
--- a/doas.conf.5
+++ b/doas.conf.5
@@ -51,15 +51,9 @@ again for some time.
 .It Ic keepenv
 The user's environment is maintained.
 The default is to reset the environment, except for the variables
-.Ev DISPLAY ,
-.Ev HOME ,
-.Ev LOGNAME ,
-.Ev MAIL ,
-.Ev PATH ,
-.Ev TERM ,
-.Ev USER
+.Ev DISPLAY
 and
-.Ev USERNAME .
+.Ev TERM .
 .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.
diff --git a/doas.h b/doas.h
index 48069db..3831dc7 100644
--- a/doas.h
+++ b/doas.h
@@ -29,7 +29,10 @@ extern struct rule **rules;
 extern int nrules;
 extern int parse_errors;
 
-char **prepenv(const struct rule *);
+struct passwd;
+
+char **prepenv(const struct rule *, const struct passwd *,
+    const struct passwd *);
 
 #define PERMIT	1
 #define DENY	2
diff --git a/env.c b/env.c
index 3e8b95d..c0f3837 100644
--- a/env.c
+++ b/env.c
@@ -24,6 +24,7 @@
 #include <err.h>
 #include <unistd.h>
 #include <errno.h>
+#include <pwd.h>
 
 #include "doas.h"
 #include "includes.h"
@@ -39,6 +40,8 @@ struct env {
 	u_int count;
 };
 
+static void fillenv(struct env *env, const char **envlist);
+
 static int
 envcmp(struct envnode *a, struct envnode *b)
 {
@@ -69,8 +72,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(const struct rule *rule)
+createenv(const struct rule *rule, const struct passwd *mypw,
+    const struct passwd *targpw)
 {
 	struct env *env;
 	u_int i;
@@ -81,6 +95,8 @@ createenv(const struct rule *rule)
 	RB_INIT(&env->root);
 	env->count = 0;
 
+	addnode(env, "DOAS_USER", mypw->pw_name);
+
 	if (rule->options & KEEPENV) {
 		extern char **environ;
 
@@ -109,6 +125,19 @@ createenv(const struct rule *rule)
 				env->count++;
 			}
 		}
+	} else {
+		static const char *copyset[] = {
+			"DISPLAY", "TERM",
+			NULL
+		};
+
+		addnode(env, "HOME", targpw->pw_dir);
+		addnode(env, "LOGNAME", targpw->pw_name);
+		addnode(env, "PATH", getenv("PATH"));
+		addnode(env, "SHELL", targpw->pw_shell);
+		addnode(env, "USER", targpw->pw_name);
+
+		fillenv(env, copyset);
 	}
 
 	return env;
@@ -187,20 +216,12 @@ fillenv(struct env *env, const char **envlist)
 }
 
 char **
-prepenv(const struct rule *rule)
+prepenv(const struct rule *rule, const struct passwd *mypw,
+    const struct passwd *targpw)
 {
-	static const char *safeset[] = {
-		"DISPLAY", "HOME", "LOGNAME", "MAIL",
-		"PATH", "TERM", "USER", "USERNAME",
-		NULL
-	};
 	struct env *env;
 
-	env = createenv(rule);
-
-	/* if we started with blank, fill some defaults then apply rules */
-	if (!(rule->options & KEEPENV))
-		fillenv(env, safeset);
+	env = createenv(rule, mypw, targpw);
 	if (rule->envlist)
 		fillenv(env, rule->envlist);
 
-- 
2.39.2