Attempt to fix problem with users being able to convince the crontab

program to read any file which is a valid crontab file.

The fix is based on that used in NetBSD and OpenBSD - we keep the
file open while the user is editing it. This means that files must
be edited in place. Cron attempts to warn you if your editor does
not do this. The fact that the file must be edited in place is also
noted in the man page.

This patch has been confirmed to work by atleast one person on
-security and has been tested locally.

Obtained from:	OpenBSD

2 files changed, 18 insertions, 9 deletions

diff --git a/usr.sbin/cron/crontab/crontab.1 b/usr.sbin/cron/crontab/crontab.1
index e904d4d..cfda038 100644
--- a/usr.sbin/cron/crontab/crontab.1
+++ b/usr.sbin/cron/crontab/crontab.1
@@ -89,7 +89,12 @@ the
 .Ev VISUAL
 or
 .Ev EDITOR
-environment variables.  After you exit
+environment variables.
+The specified editor
+.Em must
+edit the file in place;
+any editor that unlinks the file and recreates it cannot be used.
+After you exit
 from the editor, the modified crontab will be installed automatically.
 .El
 .Sh SEE ALSO
diff --git a/usr.sbin/cron/crontab/crontab.c b/usr.sbin/cron/crontab/crontab.c
index 5490f4c..0071f6a 100644
--- a/usr.sbin/cron/crontab/crontab.c
+++ b/usr.sbin/cron/crontab/crontab.c
@@ -285,7 +285,7 @@ edit_cmd() {
 	char		n[MAX_FNAME], q[MAX_TEMPSTR], *editor;
 	FILE		*f;
 	int		ch, t, x;
-	struct stat	statbuf;
+	struct stat	statbuf, fsbuf;
 	time_t		mtime;
 	WAIT_T		waiter;
 	PID_T		pid, xpid;
@@ -317,7 +317,7 @@ edit_cmd() {
 		warn("fchown");
 		goto fatal;
 	}
-	if (!(NewCrontab = fdopen(t, "w"))) {
+	if (!(NewCrontab = fdopen(t, "r+"))) {
 		warn("fdopen");
 		goto fatal;
 	}
@@ -347,14 +347,20 @@ edit_cmd() {
 		while (EOF != (ch = get_char(f)))
 			putc(ch, NewCrontab);
 	fclose(f);
-	if (fclose(NewCrontab))
+	if (fflush(NewCrontab))
 		err(ERROR_EXIT, "%s", Filename);
+	if (fstat(t, &fsbuf) < 0) {
+		warn("unable to fstat temp file");
+		goto fatal;
+	}
  again:
 	if (stat(Filename, &statbuf) < 0) {
 		warn("stat");
  fatal:		unlink(Filename);
 		exit(ERROR_EXIT);
 	}
+	if (statbuf.st_dev != fsbuf.st_dev || statbuf.st_ino != fsbuf.st_ino)
+		errx(ERROR_EXIT, "temp file must be edited in place");
 	mtime = statbuf.st_mtime;
 
 	if ((!(editor = getenv("VISUAL")))
@@ -419,15 +425,13 @@ edit_cmd() {
 		warn("stat");
 		goto fatal;
 	}
+	if (statbuf.st_dev != fsbuf.st_dev || statbuf.st_ino != fsbuf.st_ino)
+		errx(ERROR_EXIT, "temp file must be edited in place");
 	if (mtime == statbuf.st_mtime) {
 		warnx("no changes made to crontab");
 		goto remove;
 	}
 	warnx("installing new crontab");
-	if (!(NewCrontab = fopen(Filename, "r"))) {
-		warn("%s", Filename);
-		goto fatal;
-	}
 	switch (replace_cmd()) {
 	case 0:
 		break;
@@ -497,10 +501,10 @@ replace_cmd() {
 
 	/* copy the crontab to the tmp
 	 */
+	rewind(NewCrontab);
 	Set_LineNum(1)
 	while (EOF != (ch = get_char(NewCrontab)))
 		putc(ch, tmp);
-	fclose(NewCrontab);
 	ftruncate(fileno(tmp), ftell(tmp));
 	fflush(tmp);  rewind(tmp);