From owner-freebsd-audit Thu Oct 26 10:31: 4 2000 Delivered-To: freebsd-audit@freebsd.org Received: from salmon.maths.tcd.ie (salmon.maths.tcd.ie [134.226.81.11]) by hub.freebsd.org (Postfix) with SMTP id B98FA37B479; Thu, 26 Oct 2000 10:30:57 -0700 (PDT) Received: from walton.maths.tcd.ie by salmon.maths.tcd.ie with SMTP id ; 26 Oct 2000 18:30:56 +0100 (BST) To: audit@freebsd.org Cc: kris@freebsd.org, rwatson@freebsd.org Subject: Patch for crontab. X-Request-Do: Date: Thu, 26 Oct 2000 18:30:55 +0100 From: David Malone Message-ID: <200010261830.aa94658@salmon.maths.tcd.ie> Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG Can someone review this for me? It's a fix for the crontab problem based on what OpenBSD have done. We used to reopen NewCrontab after editing as root and reread the file. The patch I suggested last night just changed who we opened the file as (which works OK). What OpenBSD have done is arange to keep NewCrontab open during the edit. Then they just rewind and reread the file. This has the advantage that people just can't make a symlink to /dev/zero or /dev/random. Crontab would happily copy these byte-by-byte into the crontab spool directory. The down side to the OpenBSD patch is that it won't work with editors which unlink the file they are editing. Vi, vim, emacs, xemacs and ee all do the right thing so it looks like it isn't going to bite too many people. To try to alert people who might get caught by this I've aranged for the temp file to be stated and compared to the results of fstat for the file crontab has open. If they don't match it gives a warning and exits. This way edits made to crontab with fooedit don't silently fail. David. Index: crontab.c =================================================================== RCS file: /cvs/FreeBSD-CVS/src/usr.sbin/cron/crontab/crontab.c,v retrieving revision 1.13 diff -u -r1.13 crontab.c --- crontab.c 2000/10/15 00:35:34 1.13 +++ crontab.c 2000/10/26 16:59:47 @@ -285,7 +285,7 @@ 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 @@ warn("fchown"); goto fatal; } - if (!(NewCrontab = fdopen(t, "w"))) { + if (!(NewCrontab = fdopen(t, "r+"))) { warn("fdopen"); goto fatal; } @@ -347,14 +347,20 @@ 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 @@ 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 @@ /* 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); To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message