Date: Wed, 13 Dec 2000 10:32:56 -0500 From: Chris Faulhaber <jedgar@fxp.org> To: freebsd-audit@FreeBSD.org Subject: cron(8) fixes Message-ID: <20001213103256.A78152@peitho.fxp.org>
next in thread | raw e-mail | index | archive | help
There appear to be a few instances where buffers could be potentially overflowed. In addition, the mkprints() return value is not checked before using/free()'ing, and a warn() call appears to use an incorrect format operator (-Wall fix). Most of the sprintf() -> snprintf() changes are recommended, the other few are for consistency. -- Chris D. Faulhaber - jedgar@fxp.org - jedgar@FreeBSD.org -------------------------------------------------------- FreeBSD: The Power To Serve - http://www.FreeBSD.org Index: cron/do_command.c =================================================================== RCS file: /home/ncvs/src/usr.sbin/cron/cron/do_command.c,v retrieving revision 1.19 diff -u -r1.19 do_command.c --- cron/do_command.c 2000/07/02 04:15:15 1.19 +++ cron/do_command.c 2000/12/13 15:21:27 @@ -178,6 +178,8 @@ */ /*local*/{ char *x = mkprints((u_char *)e->cmd, strlen(e->cmd)); + if (x == NULL) + err(ERROR_EXIT, "mkprints"); log_it(usernm, getpid(), "CMD", x); free(x); Index: lib/entry.c =================================================================== RCS file: /home/ncvs/src/usr.sbin/cron/lib/entry.c,v retrieving revision 1.11 diff -u -r1.11 entry.c --- lib/entry.c 2000/05/23 13:44:00 1.11 +++ lib/entry.c 2000/12/13 15:21:27 @@ -333,7 +333,7 @@ } if (!env_get("SHELL", e->envp)) { prev_env = e->envp; - sprintf(envstr, "SHELL=%s", _PATH_BSHELL); + snprintf(envstr, sizeof(envstr), "SHELL=%s", _PATH_BSHELL); e->envp = env_set(e->envp, envstr); if (e->envp == NULL) { warn("env_set(%s)", envstr); @@ -343,7 +343,7 @@ } } prev_env = e->envp; - sprintf(envstr, "HOME=%s", pw->pw_dir); + snprintf(envstr, sizeof(envstr), "HOME=%s", pw->pw_dir); e->envp = env_set(e->envp, envstr); if (e->envp == NULL) { warn("env_set(%s)", envstr); @@ -353,7 +353,7 @@ } if (!env_get("PATH", e->envp)) { prev_env = e->envp; - sprintf(envstr, "PATH=%s", _PATH_DEFPATH); + snprintf(envstr, sizeof(envstr), "PATH=%s", _PATH_DEFPATH); e->envp = env_set(e->envp, envstr); if (e->envp == NULL) { warn("env_set(%s)", envstr); @@ -363,7 +363,7 @@ } } prev_env = e->envp; - sprintf(envstr, "%s=%s", "LOGNAME", pw->pw_name); + snprintf(envstr, sizeof(envstr), "%s=%s", "LOGNAME", pw->pw_name); e->envp = env_set(e->envp, envstr); if (e->envp == NULL) { warn("env_set(%s)", envstr); @@ -373,7 +373,7 @@ } #if defined(BSD) prev_env = e->envp; - sprintf(envstr, "%s=%s", "USER", pw->pw_name); + snprintf(envstr, sizeof(envstr), "%s=%s", "USER", pw->pw_name); e->envp = env_set(e->envp, envstr); if (e->envp == NULL) { warn("env_set(%s)", envstr); @@ -404,7 +404,7 @@ */ e->cmd = strdup(cmd); if (e->cmd == NULL) { - warn("strdup(\"%d\")", cmd); + warn("strdup(\"%s\")", cmd); env_free(e->envp); ecode = e_mem; goto eof; Index: lib/misc.c =================================================================== RCS file: /home/ncvs/src/usr.sbin/cron/lib/misc.c,v retrieving revision 1.9 diff -u -r1.9 misc.c --- lib/misc.c 2000/05/23 13:44:00 1.9 +++ lib/misc.c 2000/12/13 15:21:27 @@ -248,11 +248,12 @@ char buf[MAX_TEMPSTR]; int fd, otherpid; - (void) sprintf(pidfile, PIDFILE, PIDDIR); + (void) snprintf(pidfile, sizeof(pidfile), PIDFILE, PIDDIR); if ((-1 == (fd = open(pidfile, O_RDWR|O_CREAT, 0644))) || (NULL == (fp = fdopen(fd, "r+"))) ) { - sprintf(buf, "can't open or create %s: %s", + snprintf(buf, sizeof(buf), + "can't open or create %s: %s", pidfile, strerror(errno)); log_it("CRON", getpid(), "DEATH", buf); errx(ERROR_EXIT, "%s", buf); @@ -262,7 +263,8 @@ int save_errno = errno; fscanf(fp, "%d", &otherpid); - sprintf(buf, "can't lock %s, otherpid may be %d: %s", + snprintf(buf, sizeof(buf), + "can't lock %s, otherpid may be %d: %s", pidfile, otherpid, strerror(save_errno)); log_it("CRON", getpid(), "DEATH", buf); errx(ERROR_EXIT, "%s", buf); @@ -454,14 +456,14 @@ #if defined(SYSLOG) static int syslog_open = 0; #endif + int msglen; #if defined(LOG_FILE) /* we assume that MAX_TEMPSTR will hold the date, time, &punctuation. */ - msg = malloc(strlen(username) - + strlen(event) - + strlen(detail) - + MAX_TEMPSTR); + msglen = strlen(username) + strlen(event) + strlen(detail) + + MAX_TEMPSTR); + msg = malloc(msglen); if (msg == NULL) warnx("failed to allocate memory for log message"); @@ -475,11 +477,11 @@ } } - /* we have to sprintf() it because fprintf() doesn't always + /* we have to snprintf() it because fprintf() doesn't always * write everything out in one chunk and this has to be * atomically appended to the log file. */ - sprintf(msg, "%s (%02d/%02d-%02d:%02d:%02d-%d) %s (%s)\n", + snprintf(msg, msglen, "%s (%02d/%02d-%02d:%02d:%02d-%d) %s (%s)\n", username, t->tm_mon+1, t->tm_mday, t->tm_hour, t->tm_min, t->tm_sec, pid, event, detail); @@ -590,7 +592,7 @@ *dst++ = '^'; *dst++ = '?'; } else { /* parity character */ - sprintf(dst, "\\%03o", ch); + snprintf(dst, 5, "\\%03o", ch); dst += 4; } } To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20001213103256.A78152>