3 files changed, 79 insertions, 34 deletions

diff --git a/usr.sbin/cron/cron/do_command.c b/usr.sbin/cron/cron/do_command.c
index 214baf2133ed..4dcbce2c1165 100644
--- a/usr.sbin/cron/cron/do_command.c
+++ b/usr.sbin/cron/cron/do_command.c
@@ -97,6 +97,7 @@ child_process(e, u)
 	register FILE	*mail;
 	register int	bytes = 1;
 	int		status = 0;
+	const char	*homedir = NULL;
 # if defined(LOGIN_CAP)
 	struct passwd	*pwd;
 	login_cap_t *lc;
@@ -287,6 +288,14 @@ child_process(e, u)
 			pwd = getpwuid(e->uid);
 		lc = NULL;
 		if (pwd != NULL) {
+			if (pwd->pw_dir != NULL
+			    && pwd->pw_dir[0] != '\0') {
+				homedir = strdup(pwd->pw_dir);
+				if (homedir == NULL) {
+					warn("strdup");
+					_exit(ERROR_EXIT);
+				}
+			}
 			pwd->pw_gid = e->gid;
 			if (e->class != NULL)
 				lc = login_getclass(e->class);
@@ -330,17 +339,38 @@ child_process(e, u)
 		if (lc != NULL)
 			login_close(lc);
 #endif
-		chdir(env_get("HOME", e->envp));
 
-		/* exec the command.
+		/* For compatibility, we chdir to the value of HOME if it was
+		 * specified explicitly in the crontab file, but not if it was
+		 * set in the environment by some other mechanism. We chdir to
+		 * the homedir given by the pw entry otherwise.
+		 *
+		 * If !LOGIN_CAP, then HOME is always set in e->envp.
+		 *
+		 * XXX: probably should also consult PAM.
+		 */
+		{
+			char	*new_home = env_get("HOME", e->envp);
+			if (new_home != NULL && new_home[0] != '\0')
+				chdir(new_home);
+			else if (homedir != NULL)
+				chdir(homedir);
+			else
+				chdir("/");
+		}
+
+		/* exec the command. Note that SHELL is not respected from
+		 * either login.conf or pw_shell, only an explicit setting
+		 * in the crontab. (default of _PATH_BSHELL is supplied when
+		 * setting up the entry)
 		 */
 		{
 			char	*shell = env_get("SHELL", e->envp);
 			char	**p;
 
-			/* Apply the environment from the entry, overriding existing
-			 * values (this will always set PATH, LOGNAME, etc.) putenv
-			 * should not fail unless malloc does.
+			/* Apply the environment from the entry, overriding
+			 * existing values (this will always set LOGNAME and
+			 * SHELL). putenv should not fail unless malloc does.
 			 */
 			for (p = e->envp; *p; ++p) {
 				if (putenv(*p) != 0) {
@@ -349,6 +379,25 @@ child_process(e, u)
 				}
 			}
 
+			/* HOME in login.conf overrides pw, and HOME in the
+			 * crontab overrides both. So set pw's value only if
+			 * nothing was already set (overwrite==0).
+			 */
+			if (homedir != NULL
+			    && setenv("HOME", homedir, 0) < 0) {
+				warn("setenv(HOME)");
+				_exit(ERROR_EXIT);
+			}
+
+			/* PATH in login.conf is respected, but the crontab
+			 * overrides; set a default value only if nothing
+			 * already set.
+			 */
+			if (setenv("PATH", _PATH_DEFPATH, 0) < 0) {
+				warn("setenv(PATH)");
+				_exit(ERROR_EXIT);
+			}
+
 # if DEBUGGING
 			if (DebugFlags & DTEST) {
 				fprintf(stderr,
diff --git a/usr.sbin/cron/crontab/crontab.5 b/usr.sbin/cron/crontab/crontab.5
index adc2b2b5ae95..44c3c3a36f0f 100644
--- a/usr.sbin/cron/crontab/crontab.5
+++ b/usr.sbin/cron/crontab/crontab.5
@@ -17,7 +17,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd January 19, 2020
+.Dd March 29, 2020
 .Dt CRONTAB 5
 .Os
 .Sh NAME
@@ -72,9 +72,6 @@ daemon.
 .Ev SHELL
 is set to
 .Pa /bin/sh ,
-.Ev PATH
-is set to
-.Pa /sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin ,
 and
 .Ev LOGNAME
 and
@@ -83,12 +80,22 @@ are set from the
 .Pa /etc/passwd
 line of the crontab's owner.
 In addition, the environment variables of the
-user's login class, with the exception of
-.Ev PATH ,
-will be set from
+user's login class will be set from
 .Pa /etc/login.conf.db
 and
 .Pa ~/.login_conf .
+(A setting of
+.Ev HOME
+in the login class will override the value from
+.Pa /etc/passwd ,
+but will not change the current directory when the command is
+invoked, which can only be overridden with an explicit setting of
+.Ev HOME
+within the crontab file itself.)
+If
+.Ev PATH
+is not set by any other means, it is defaulted to
+.Pa /sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin .
 .Ev HOME ,
 .Ev PATH
 and
@@ -109,17 +116,11 @@ On these systems,
 .Ev USER
 will be set also).
 .Pp
-In addition to
-.Ev LOGNAME ,
-.Ev HOME ,
-.Ev PATH ,
-and
-.Ev SHELL ,
+If
 .Xr cron 8
-will look at
-.Ev MAILTO
-if it has any reason to send mail as a result of running
-commands in ``this'' crontab.
+has any reason to send mail as a result of running commands in
+``this'' crontab, it will respect the following settings which may be
+defined in the crontab (but which are not taken from the login class).
 If
 .Ev MAILTO
 is defined (and non-empty), mail is
diff --git a/usr.sbin/cron/lib/entry.c b/usr.sbin/cron/lib/entry.c
index 8c579b8f511d..66ead885bea8 100644
--- a/usr.sbin/cron/lib/entry.c
+++ b/usr.sbin/cron/lib/entry.c
@@ -369,7 +369,8 @@ load_entry(file, error_func, pw, envp)
 	e->gid = pw->pw_gid;
 
 	/* copy and fix up environment.  some variables are just defaults and
-	 * others are overrides.
+	 * others are overrides; we process only the overrides here, defaults
+	 * are handled in do_command after login.conf is processed.
 	 */
 	e->envp = env_copy(envp);
 	if (e->envp == NULL) {
@@ -388,6 +389,10 @@ load_entry(file, error_func, pw, envp)
 			goto eof;
 		}
 	}
+	/* If LOGIN_CAP, this is deferred to do_command where the login class
+	 * is processed. If !LOGIN_CAP, do it here.
+	 */
+#ifndef LOGIN_CAP
 	if (!env_get("HOME", e->envp)) {
 		prev_env = e->envp;
 		sprintf(envstr, "HOME=%s", pw->pw_dir);
@@ -399,17 +404,7 @@ load_entry(file, error_func, pw, envp)
 			goto eof;
 		}
 	}
-	if (!env_get("PATH", e->envp)) {
-		prev_env = e->envp;
-		sprintf(envstr, "PATH=%s", _PATH_DEFPATH);
-		e->envp = env_set(e->envp, envstr);
-		if (e->envp == NULL) {
-			warn("env_set(%s)", envstr);
-			env_free(prev_env);
-			ecode = e_mem;
-			goto eof;
-		}
-	}
+#endif
 	prev_env = e->envp;
 	sprintf(envstr, "%s=%s", "LOGNAME", pw->pw_name);
 	e->envp = env_set(e->envp, envstr);