From: Todd C. Miller <Todd.Miller@courtesan.com>
Date: Tue, 6 Oct 2015 16:25:43 +0000 (-0600)
Subject: For env_reset, SHELL should be set based on the target user, not
X-Git-Tag: SUDO_1_8_15^2~18
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=588460405f1622e1852791167ccb31e27c3dc658;p=sudo

For env_reset, SHELL should be set based on the target user, not
the invoking user unless preserved via env_keep.
---

diff --git a/NEWS b/NEWS
index b04b77ffc..3c206ca15 100644
--- a/NEWS
+++ b/NEWS
@@ -67,6 +67,11 @@ What's new in Sudo 1.8.15
 
  * Hungarian and Slovak translations for sudoers from translationproject.org.
 
+ * Previously, when env_reset was enabled (the default) and the -s
+   option was not used, the SHELL environment variable was set to the
+   shell of the invoking user.  Now, when env_reset is enabled and
+   the -s option is not used, SHELL is set based on the target user.
+
 What's new in Sudo 1.8.14p3
 
  * Fixed a bug introduced in sudo 1.8.14p2 that prevented sudo
diff --git a/doc/UPGRADE b/doc/UPGRADE
index 358f2d745..2678d1a99 100644
--- a/doc/UPGRADE
+++ b/doc/UPGRADE
@@ -3,6 +3,12 @@ Notes on upgrading from an older release
 
 o Upgrading from a version prior to 1.8.15:
 
+    Prior to version 1.8.15, when env_reset was enabled (the default)
+    and the -s option was not used, the SHELL environment variable
+    was set to the shell of the invoking user.  In 1.8.15 and above,
+    when env_reset is enabled and the -s option is not used, SHELL
+    is set based on the target user.
+
     When editing files with sudoedit, symbolic links will no longer
     be followed by default.  The old behavior can be restored by
     enabling the sudoedit_follow option in sudoers or on a per-command
diff --git a/plugins/sudoers/env.c b/plugins/sudoers/env.c
index 39d24dfb0..90652508a 100644
--- a/plugins/sudoers/env.c
+++ b/plugins/sudoers/env.c
@@ -939,8 +939,6 @@ rebuild_env(void)
 	    CHECK_SETENV2("USERNAME", runas_pw->pw_name,
 		ISSET(didvar, DID_USERNAME), true);
 	} else {
-	    if (!ISSET(didvar, DID_SHELL))
-		CHECK_SETENV2("SHELL", sudo_user.pw->pw_shell, false, true);
 	    /* We will set LOGNAME later in the def_set_logname case. */
 	    if (!def_set_logname) {
 		if (!ISSET(didvar, DID_LOGNAME))
@@ -984,6 +982,8 @@ rebuild_env(void)
 	    if (!env_should_delete(*ep)) {
 		if (strncmp(*ep, "SUDO_PS1=", 9) == 0)
 		    ps1 = *ep + 5;
+		else if (strncmp(*ep, "SHELL=", 6) == 0)
+		    SET(didvar, DID_SHELL);
 		else if (strncmp(*ep, "PATH=", 5) == 0)
 		    SET(didvar, DID_PATH);
 		else if (strncmp(*ep, "TERM=", 5) == 0)
@@ -1039,7 +1039,9 @@ rebuild_env(void)
     if (reset_home)
 	CHECK_SETENV2("HOME", runas_pw->pw_dir, true, true);
 
-    /* Provide default values for $TERM and $PATH if they are not set. */
+    /* Provide default values for $SHELL, $TERM and $PATH if not set. */
+    if (!ISSET(didvar, DID_SHELL))
+	CHECK_SETENV2("SHELL", runas_pw->pw_shell, false, false);
     if (!ISSET(didvar, DID_TERM))
 	CHECK_PUTENV("TERM=unknown", false, false);
     if (!ISSET(didvar, DID_PATH))