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
This commit is contained in:
David Malone 2000-11-06 11:17:37 +00:00
parent d6801b5c51
commit 25e9ca2b19
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=68388
2 changed files with 18 additions and 9 deletions

View File

@ -89,7 +89,12 @@ the
.Ev VISUAL .Ev VISUAL
or or
.Ev EDITOR .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. from the editor, the modified crontab will be installed automatically.
.El .El
.Sh SEE ALSO .Sh SEE ALSO

View File

@ -285,7 +285,7 @@ edit_cmd() {
char n[MAX_FNAME], q[MAX_TEMPSTR], *editor; char n[MAX_FNAME], q[MAX_TEMPSTR], *editor;
FILE *f; FILE *f;
int ch, t, x; int ch, t, x;
struct stat statbuf; struct stat statbuf, fsbuf;
time_t mtime; time_t mtime;
WAIT_T waiter; WAIT_T waiter;
PID_T pid, xpid; PID_T pid, xpid;
@ -317,7 +317,7 @@ edit_cmd() {
warn("fchown"); warn("fchown");
goto fatal; goto fatal;
} }
if (!(NewCrontab = fdopen(t, "w"))) { if (!(NewCrontab = fdopen(t, "r+"))) {
warn("fdopen"); warn("fdopen");
goto fatal; goto fatal;
} }
@ -347,14 +347,20 @@ edit_cmd() {
while (EOF != (ch = get_char(f))) while (EOF != (ch = get_char(f)))
putc(ch, NewCrontab); putc(ch, NewCrontab);
fclose(f); fclose(f);
if (fclose(NewCrontab)) if (fflush(NewCrontab))
err(ERROR_EXIT, "%s", Filename); err(ERROR_EXIT, "%s", Filename);
if (fstat(t, &fsbuf) < 0) {
warn("unable to fstat temp file");
goto fatal;
}
again: again:
if (stat(Filename, &statbuf) < 0) { if (stat(Filename, &statbuf) < 0) {
warn("stat"); warn("stat");
fatal: unlink(Filename); fatal: unlink(Filename);
exit(ERROR_EXIT); 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; mtime = statbuf.st_mtime;
if ((!(editor = getenv("VISUAL"))) if ((!(editor = getenv("VISUAL")))
@ -419,15 +425,13 @@ edit_cmd() {
warn("stat"); warn("stat");
goto fatal; 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) { if (mtime == statbuf.st_mtime) {
warnx("no changes made to crontab"); warnx("no changes made to crontab");
goto remove; goto remove;
} }
warnx("installing new crontab"); warnx("installing new crontab");
if (!(NewCrontab = fopen(Filename, "r"))) {
warn("%s", Filename);
goto fatal;
}
switch (replace_cmd()) { switch (replace_cmd()) {
case 0: case 0:
break; break;
@ -497,10 +501,10 @@ replace_cmd() {
/* copy the crontab to the tmp /* copy the crontab to the tmp
*/ */
rewind(NewCrontab);
Set_LineNum(1) Set_LineNum(1)
while (EOF != (ch = get_char(NewCrontab))) while (EOF != (ch = get_char(NewCrontab)))
putc(ch, tmp); putc(ch, tmp);
fclose(NewCrontab);
ftruncate(fileno(tmp), ftell(tmp)); ftruncate(fileno(tmp), ftell(tmp));
fflush(tmp); rewind(tmp); fflush(tmp); rewind(tmp);