Module Name: src Committed By: christos Date: Mon Dec 15 16:45:26 UTC 2014
Modified Files: src/external/bsd/cron/dist: crontab.c Log Message: - Don't allow bypassing file size limits with crontabs from stdin. - Allow signals while reading the user crontab file; doing "crontab -" does not let you abort otherwise, and doing ^Z, kill %1 leaves turds in /var/cron/tabs To generate a diff of this commit: cvs rdiff -u -r1.11 -r1.12 src/external/bsd/cron/dist/crontab.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/external/bsd/cron/dist/crontab.c diff -u src/external/bsd/cron/dist/crontab.c:1.11 src/external/bsd/cron/dist/crontab.c:1.12 --- src/external/bsd/cron/dist/crontab.c:1.11 Sun Sep 7 09:34:12 2014 +++ src/external/bsd/cron/dist/crontab.c Mon Dec 15 11:45:26 2014 @@ -1,4 +1,4 @@ -/* $NetBSD: crontab.c,v 1.11 2014/09/07 13:34:12 joerg Exp $ */ +/* $NetBSD: crontab.c,v 1.12 2014/12/15 16:45:26 christos Exp $ */ /* Copyright 1988,1990,1993,1994 by Paul Vixie * All rights reserved @@ -25,7 +25,7 @@ #if 0 static char rcsid[] = "Id: crontab.c,v 1.12 2004/01/23 18:56:42 vixie Exp"; #else -__RCSID("$NetBSD: crontab.c,v 1.11 2014/09/07 13:34:12 joerg Exp $"); +__RCSID("$NetBSD: crontab.c,v 1.12 2014/12/15 16:45:26 christos Exp $"); #endif #endif @@ -169,7 +169,7 @@ compare_time(const struct stat *st, cons { struct timespec ts1[2]; get_time(st, ts1); - + return ts1[1].tv_sec == ts2[1].tv_sec #if defined(HAVE_UTIMENSAT) && ts1[1].tv_nsec == ts2[1].tv_nsec @@ -286,7 +286,7 @@ skip_header(int *pch, FILE *f) { int ch; int x; - + /* ignore the top few comments since we probably put them there. */ for (x = 0; x < NHEADER_LINES; x++) { @@ -554,15 +554,81 @@ edit_cmd(void) { log_it(RealUser, Pid, "END EDIT", User); } +static size_t +getmaxtabsize(void) +{ + char n2[MAX_FNAME]; + FILE *fmaxtabsize; + size_t maxtabsize; + + /* Make sure that the crontab is not an unreasonable size. + * + * XXX This is subject to a race condition--the user could + * add stuff to the file after we've checked the size but + * before we slurp it in and write it out. We can't just move + * the test to test the temp file we later create, because by + * that time we've already filled up the crontab disk. Probably + * the right thing to do is to do a bytecount in the copy loop + * rather than stating the file we're about to read. + */ + (void)snprintf(n2, sizeof(n2), "%s/%s", CRONDIR, MAXTABSIZE_FILE); + if ((fmaxtabsize = fopen(n2, "r")) != NULL) { + if (fgets(n2, (int)sizeof(n2), fmaxtabsize) == NULL) { + maxtabsize = 0; + } else { + maxtabsize = (size_t)atoi(n2); + } + (void)fclose(fmaxtabsize); + } else { + maxtabsize = MAXTABSIZE_DEFAULT; + } + return maxtabsize; +} + +static int +checkmaxtabsize(FILE *fp, size_t maxtabsize) +{ + struct stat statbuf; + + if (fstat(fileno(fp), &statbuf)) { + warn("error stat'ing crontab input"); + return 0; + } + if ((uintmax_t)statbuf.st_size > (uintmax_t)maxtabsize) { + warnx("%ju bytes is larger than the maximum size of %ju bytes", + (uintmax_t)statbuf.st_size, (uintmax_t)maxtabsize); + return 0; + } + return 1; +} + +static void +cleanTempFile(void) +{ + + if (TempFilename[0]) { + (void) unlink(TempFilename); + TempFilename[0] = '\0'; + } +} + +static void +bail(int signo) +{ + + cleanTempFile(); + errx(ERROR_EXIT, "Exiting on signal %d", signo); +} + /* returns 0 on success * -1 on syntax error * -2 on install error */ static int replace_cmd(void) { - char n[MAX_FNAME], n2[MAX_FNAME], envstr[MAX_ENVSTR]; + char n[MAX_FNAME], envstr[MAX_ENVSTR]; int lastch; - FILE *tmp, *fmaxtabsize; + FILE *tmp = NULL; int ch, eof, fd; int error = 0; entry *e; @@ -570,8 +636,7 @@ replace_cmd(void) { uid_t file_owner; time_t now = time(NULL); char **envp = env_init(); - size_t maxtabsize; - struct stat statbuf; + size_t i, maxtabsize; if (envp == NULL) { warn("Cannot allocate memory."); @@ -584,71 +649,62 @@ replace_cmd(void) { warnx("path too long"); return (-2); } + + /* Interruptible while doing I/O */ + ohup = signal(SIGHUP, bail); + oint = signal(SIGINT, bail); + oquit = signal(SIGQUIT, bail); + oabrt = signal(SIGABRT, bail); + if ((fd = mkstemp(TempFilename)) == -1 || !(tmp = fdopen(fd, "w+"))) { warn("cannot create `%s'", TempFilename); - if (fd != -1) { + if (fd != -1) (void)close(fd); - (void)unlink(TempFilename); - } - TempFilename[0] = '\0'; - return (-2); + error = -2; + goto done; } - ohup = signal(SIGHUP, SIG_IGN); - oint = signal(SIGINT, SIG_IGN); - oquit = signal(SIGQUIT, SIG_IGN); - oabrt = signal(SIGABRT, SIG_IGN); - - /* Make sure that the crontab is not an unreasonable size. - * - * XXX This is subject to a race condition--the user could - * add stuff to the file after we've checked the size but - * before we slurp it in and write it out. We can't just move - * the test to test the temp file we later create, because by - * that time we've already filled up the crontab disk. Probably - * the right thing to do is to do a bytecount in the copy loop - * rather than stating the file we're about to read. - */ - (void)snprintf(n2, sizeof(n2), "%s/%s", CRONDIR, MAXTABSIZE_FILE); - if ((fmaxtabsize = fopen(n2, "r")) != NULL) { - if (fgets(n2, (int)sizeof(n2), fmaxtabsize) == NULL) { - maxtabsize = 0; - } else { - maxtabsize = (size_t)atoi(n2); - } - (void)fclose(fmaxtabsize); - } else { - maxtabsize = MAXTABSIZE_DEFAULT; - } + maxtabsize = getmaxtabsize(); - if (fstat(fileno(NewCrontab), &statbuf)) { - warn("error stat'ing crontab input"); - error = -2; - goto done; - } - if ((uintmax_t)statbuf.st_size > (uintmax_t)maxtabsize) { - warnx("%ld bytes is larger than the maximum size of %ld bytes", - (long) statbuf.st_size, (long) maxtabsize); - error = -2; - goto done; + /* This does not work for stdin, so we'll also check later */ + if (!checkmaxtabsize(NewCrontab, maxtabsize)) { + error = -2; + goto done; } /* write a signature at the top of the file. * * VERY IMPORTANT: make sure NHEADER_LINES agrees with this code. */ - (void)fprintf(tmp, "# DO NOT EDIT THIS FILE - edit the master and reinstall.\n"); - (void)fprintf(tmp, "# (%s installed on %-24.24s)\n", Filename, ctime(&now)); - (void)fprintf(tmp, "# (Cron version %s -- %s)\n", CRON_VERSION, "$NetBSD: crontab.c,v 1.11 2014/09/07 13:34:12 joerg Exp $"); + (void)fprintf(tmp, + "# DO NOT EDIT THIS FILE - edit the master and reinstall.\n"); + (void)fprintf(tmp, + "# (%s installed on %-24.24s)\n", Filename, ctime(&now)); + (void)fprintf(tmp, + "# (Cron version %s -- %s)\n", CRON_VERSION, + "$NetBSD: crontab.c,v 1.12 2014/12/15 16:45:26 christos Exp $"); /* copy the crontab to the tmp */ (void)rewind(NewCrontab); Set_LineNum(1); lastch = EOF; - while (EOF != (ch = get_char(NewCrontab))) + for (i = 0; i < maxtabsize && EOF != (ch = get_char(NewCrontab)); i++) (void)putc(lastch = (char)ch, tmp); + /* Non-Interruptible while installing */ + (void)signal(SIGHUP, SIG_IGN); + (void)signal(SIGINT, SIG_IGN); + (void)signal(SIGQUIT, SIG_IGN); + (void)signal(SIGABRT, SIG_IGN); + + if (i == maxtabsize) { + warnx("is larger than the maximum size of %ju bytes", + (uintmax_t)maxtabsize); + error = -2; + goto done; + } + if (lastch != EOF && lastch != '\n') { warnx("missing trailing newline in `%s'", Filename); error = -1; @@ -666,7 +722,6 @@ replace_cmd(void) { (void)fflush(tmp); if (ferror(tmp)) { - (void)fclose(tmp); warn("error while writing new crontab to `%s'", TempFilename); error = -2; goto done; @@ -704,7 +759,6 @@ replace_cmd(void) { if (CheckErrorCount != 0) { warnx("errors in crontab file, can't install."); - (void)fclose(tmp); error = -1; goto done; } @@ -716,15 +770,17 @@ replace_cmd(void) { #else error = chown(TempFilename, file_owner, (gid_t)-1); #endif - if (error < OK) { - warn("cannot chown `%s'", TempFilename); - (void)fclose(tmp); + + if (fclose(tmp) == EOF) { + warn("error closing file"); error = -2; + tmp = NULL; goto done; } + tmp = NULL; - if (fclose(tmp) == EOF) { - warn("error closing file"); + if (error < OK) { + warn("cannot chown `%s'", TempFilename); error = -2; goto done; } @@ -749,10 +805,9 @@ done: (void)signal(SIGINT, oint); (void)signal(SIGQUIT, oquit); (void)signal(SIGABRT, oabrt); - if (TempFilename[0]) { - (void) unlink(TempFilename); - TempFilename[0] = '\0'; - } + if (tmp != NULL) + (void)fclose(tmp); + cleanTempFile(); return (error); }