From dc2bdcbddf8a4852ac0e0efe128b1b0b5c6ee6b5 Mon Sep 17 00:00:00 2001 From: Kris Kennaway Date: Sat, 1 Sep 2001 07:57:35 +0000 Subject: [PATCH] Fix some unsafe signal handlers, and be careful not to overflow on fd_set. Fix some string buffer operations. Based on: OpenBSD Reviewed by: audit MFC after: 2 weeks --- usr.sbin/syslogd/syslogd.c | 240 +++++++++++++++++++++---------------- 1 file changed, 137 insertions(+), 103 deletions(-) diff --git a/usr.sbin/syslogd/syslogd.c b/usr.sbin/syslogd/syslogd.c index d300c848c062..f05f4331bfd8 100644 --- a/usr.sbin/syslogd/syslogd.c +++ b/usr.sbin/syslogd/syslogd.c @@ -282,6 +282,8 @@ int LogFacPri = 0; /* Put facility and priority in log message: */ /* 0=no, 1=numeric, 2=names */ int KeepKernFac = 0; /* Keep remotely logged kernel facility */ +volatile sig_atomic_t MarkSet, WantDie; + int allowaddr __P((char *)); void cfline __P((char *, struct filed *, char *, char *)); char *cvthname __P((struct sockaddr *)); @@ -289,6 +291,7 @@ void deadq_enter __P((pid_t, const char *)); int deadq_remove __P((pid_t)); int decode __P((const char *, CODE *)); void die __P((int)); +void dodie __P((int)); void domark __P((int)); void fprintlog __P((struct filed *, int, char *)); int* socksetup __P((int)); @@ -296,6 +299,7 @@ void init __P((int)); void logerror __P((const char *)); void logmsg __P((int, char *, char *, int)); void log_deadchild __P((pid_t, int, const char *)); +void markit __P((void)); void printline __P((char *, char *)); void printsys __P((char *)); int p_open __P((char *, pid_t *)); @@ -314,9 +318,10 @@ main(argc, argv) int argc; char *argv[]; { - int ch, i, l; + int ch, i, fdsrmax = 0, l; struct sockaddr_un sunx, fromunix; struct sockaddr_storage frominet; + fd_set *fdsr = NULL; FILE *fp; char *hname, line[MAXLINE + 1]; struct timeval tv, *tvp; @@ -397,11 +402,12 @@ main(argc, argv) endservent(); consfile.f_type = F_CONSOLE; - (void)strcpy(consfile.f_un.f_fname, ctty + sizeof _PATH_DEV - 1); - (void)strcpy(bootfile, getbootfile()); - (void)signal(SIGTERM, die); - (void)signal(SIGINT, Debug ? die : SIG_IGN); - (void)signal(SIGQUIT, Debug ? die : SIG_IGN); + (void)strlcpy(consfile.f_un.f_fname, ctty + sizeof _PATH_DEV - 1, + sizeof(consfile.f_un.f_fname)); + (void)strlcpy(bootfile, getbootfile(), sizeof(bootfile)); + (void)signal(SIGTERM, dodie); + (void)signal(SIGINT, Debug ? dodie : SIG_IGN); + (void)signal(SIGQUIT, Debug ? dodie : SIG_IGN); /* * We don't want the SIGCHLD and SIGHUP handlers to interfere * with each other; they are likely candidates for being called @@ -426,7 +432,7 @@ main(argc, argv) for (i = 0; i < nfunix; i++) { memset(&sunx, 0, sizeof(sunx)); sunx.sun_family = AF_UNIX; - (void)strncpy(sunx.sun_path, funixn[i], sizeof(sunx.sun_path)); + (void)strlcpy(sunx.sun_path, funixn[i], sizeof(sunx.sun_path)); funix[i] = socket(AF_UNIX, SOCK_DGRAM, 0); if (funix[i] < 0 || bind(funix[i], (struct sockaddr *)&sunx, @@ -484,53 +490,65 @@ main(argc, argv) tvp = &tv; tv.tv_sec = tv.tv_usec = 0; - for (;;) { - fd_set readfds; - int nfds = 0; - - FD_ZERO(&readfds); - if (fklog != -1) { - FD_SET(fklog, &readfds); - if (fklog > nfds) - nfds = fklog; + if (fklog != -1 && fklog > fdsrmax) + fdsrmax = fklog; + if (finet && !SecureMode) { + for (i = 0; i < *finet; i++) { + if (finet[i+1] != -1 && finet[i+1] > fdsrmax) + fdsrmax = finet[i+1]; } + } + for (i = 0; i < nfunix; i++) { + if (funix[i] != -1 && funix[i] > fdsrmax) + fdsrmax = funix[i]; + } + + fdsr = (fd_set *)calloc(howmany(fdsrmax+1, NFDBITS), + sizeof(fd_mask)); + if (fdsr == NULL) + errx(1, "calloc fd_set"); + + for (;;) { + if (MarkSet) + markit(); + if (WantDie) + die(WantDie); + + bzero(fdsr, howmany(fdsrmax+1, NFDBITS) * + sizeof(fd_mask)); + + if (fklog != -1) + FD_SET(fklog, fdsr); if (finet && !SecureMode) { for (i = 0; i < *finet; i++) { - FD_SET(finet[i+1], &readfds); - if (finet[i+1] > nfds) - nfds = finet[i+1]; + if (finet[i+1] != -1) + FD_SET(finet[i+1], fdsr); } } for (i = 0; i < nfunix; i++) { - if (funix[i] != -1) { - FD_SET(funix[i], &readfds); - if (funix[i] > nfds) - nfds = funix[i]; - } + if (funix[i] != -1) + FD_SET(funix[i], fdsr); } - /*dprintf("readfds = %#x\n", readfds);*/ - nfds = select(nfds+1, &readfds, (fd_set *)NULL, - (fd_set *)NULL, tvp); - if (nfds == 0) { + i = select(fdsrmax+1, fdsr, NULL, NULL, tvp); + switch (i) { + case 0: if (tvp) { tvp = NULL; if (ppid != 1) kill(ppid, SIGALRM); } continue; - } - if (nfds < 0) { + case -1: if (errno != EINTR) logerror("select"); continue; } - /*dprintf("got a message (%d, %#x)\n", nfds, readfds);*/ - if (fklog != -1 && FD_ISSET(fklog, &readfds)) + if (fklog != -1 && FD_ISSET(fklog, fdsr)) readklog(); if (finet && !SecureMode) { for (i = 0; i < *finet; i++) { - if (FD_ISSET(finet[i+1], &readfds)) { + if (FD_ISSET(finet[i+1], fdsr)) { len = sizeof(frominet); l = recvfrom(finet[i+1], line, MAXLINE, 0, (struct sockaddr *)&frominet, @@ -547,7 +565,7 @@ main(argc, argv) } } for (i = 0; i < nfunix; i++) { - if (funix[i] != -1 && FD_ISSET(funix[i], &readfds)) { + if (funix[i] != -1 && FD_ISSET(funix[i], fdsr)) { len = sizeof(fromunix); l = recvfrom(funix[i], line, MAXLINE, 0, (struct sockaddr *)&fromunix, &len); @@ -559,6 +577,8 @@ main(argc, argv) } } } + if (fdsr) + free(fdsr); } static void @@ -837,7 +857,7 @@ logmsg(pri, msg, from, flags) if ((flags & MARK) == 0 && msglen == f->f_prevlen && !strcmp(msg, f->f_prevline) && !strcasecmp(from, f->f_prevhost)) { - (void)strncpy(f->f_lasttime, timestamp, 15); + (void)strlcpy(f->f_lasttime, timestamp, 16); f->f_prevcount++; dprintf("msg repeated %d times, %ld sec of %d\n", f->f_prevcount, (long)(now - f->f_time), @@ -858,13 +878,12 @@ logmsg(pri, msg, from, flags) fprintlog(f, 0, (char *)NULL); f->f_repeatcount = 0; f->f_prevpri = pri; - (void)strncpy(f->f_lasttime, timestamp, 15); - (void)strncpy(f->f_prevhost, from, - sizeof(f->f_prevhost)-1); - f->f_prevhost[sizeof(f->f_prevhost)-1] = '\0'; + (void)strlcpy(f->f_lasttime, timestamp, 16); + (void)strlcpy(f->f_prevhost, from, + sizeof(f->f_prevhost)); if (msglen < MAXSVLINE) { f->f_prevlen = msglen; - (void)strcpy(f->f_prevline, msg); + (void)strlcpy(f->f_prevline, msg, sizeof(f->f_prevline)); fprintlog(f, flags, (char *)NULL); } else { f->f_prevline[0] = 0; @@ -963,8 +982,8 @@ fprintlog(f, flags, msg) v->iov_len = strlen(msg); } else if (f->f_prevcount > 1) { v->iov_base = repbuf; - v->iov_len = sprintf(repbuf, "last message repeated %d times", - f->f_prevcount); + v->iov_len = snprintf(repbuf, sizeof repbuf, + "last message repeated %d times", f->f_prevcount); } else { v->iov_base = f->f_prevline; v->iov_len = f->f_prevlen; @@ -1121,8 +1140,7 @@ wallmsg(f, iov) while (fread((char *)&ut, sizeof(ut), 1, uf) == 1) { if (ut.ut_name[0] == '\0') continue; - strncpy(line, ut.ut_line, sizeof(ut.ut_line)); - line[sizeof(ut.ut_line)] = '\0'; + (void)strlcpy(line, ut.ut_line, sizeof(line)); if (f->f_type == F_WALL) { if ((p = ttymsg(iov, 7, line, TTYMSGTIME)) != NULL) { errno = 0; /* already in msg */ @@ -1223,57 +1241,20 @@ cvthname(f) return (hname); } +void +dodie(signo) + int signo; +{ + + WantDie = signo; +} + void domark(signo) int signo; { - struct filed *f; - dq_t q; - now = time((time_t *)NULL); - MarkSeq += TIMERINTVL; - if (MarkSeq >= MarkInterval) { - logmsg(LOG_INFO, "-- MARK --", LocalHostName, ADDDATE|MARK); - MarkSeq = 0; - } - - for (f = Files; f; f = f->f_next) { - if (f->f_prevcount && now >= REPEATTIME(f)) { - dprintf("flush %s: repeated %d times, %d sec.\n", - TypeNames[f->f_type], f->f_prevcount, - repeatinterval[f->f_repeatcount]); - fprintlog(f, 0, (char *)NULL); - BACKOFF(f); - } - } - - /* Walk the dead queue, and see if we should signal somebody. */ - for (q = TAILQ_FIRST(&deadq_head); q != NULL; q = TAILQ_NEXT(q, dq_entries)) - switch (q->dq_timeout) { - case 0: - /* Already signalled once, try harder now. */ - if (kill(q->dq_pid, SIGKILL) != 0) - (void)deadq_remove(q->dq_pid); - break; - - case 1: - /* - * Timed out on dead queue, send terminate - * signal. Note that we leave the removal - * from the dead queue to reapchild(), which - * will also log the event (unless the process - * didn't even really exist, in case we simply - * drop it from the dead queue). - */ - if (kill(q->dq_pid, SIGTERM) != 0) - (void)deadq_remove(q->dq_pid); - /* FALLTHROUGH */ - - default: - q->dq_timeout--; - } - - (void)alarm(TIMERINTVL); + MarkSet = 1; } /* @@ -1316,7 +1297,7 @@ die(signo) Initialized = was_initialized; if (signo) { dprintf("syslogd: exiting on signal %d\n", signo); - (void)sprintf(buf, "exiting on signal %d", signo); + (void)snprintf(buf, sizeof(buf), "exiting on signal %d", signo); errno = 0; logerror(buf); } @@ -1406,8 +1387,8 @@ init(signo) * Foreach line in the conf table, open that file. */ f = NULL; - strcpy(host, "*"); - strcpy(prog, "*"); + (void)strlcpy(host, "*", sizeof(host)); + (void)strlcpy(prog, "*", sizeof(prog)); while (fgets(cline, sizeof(cline), cf) != NULL) { /* * check for end-of-section, comments, strip off trailing @@ -1427,7 +1408,7 @@ init(signo) host[0] = *p++; while (isspace(*p)) p++; if ((!*p) || (*p == '*')) { - strcpy(host, "*"); + (void)strlcpy(host, "*", sizeof(host)); continue; } if (*p == '@') @@ -1444,7 +1425,7 @@ init(signo) p++; while (isspace(*p)) p++; if ((!*p) || (*p == '*')) { - strcpy(prog, "*"); + (void)strlcpy(prog, "*", sizeof(prog)); continue; } for (i = 0; i < NAME_MAX; i++) { @@ -1652,9 +1633,8 @@ cfline(line, f, prog, host) switch (*p) { case '@': - (void)strncpy(f->f_un.f_forw.f_hname, ++p, - sizeof(f->f_un.f_forw.f_hname)-1); - f->f_un.f_forw.f_hname[sizeof(f->f_un.f_forw.f_hname)-1] = '\0'; + (void)strlcpy(f->f_un.f_forw.f_hname, ++p, + sizeof(f->f_un.f_forw.f_hname)); memset(&hints, 0, sizeof(hints)); hints.ai_family = family; hints.ai_socktype = SOCK_DGRAM; @@ -1679,16 +1659,17 @@ cfline(line, f, prog, host) f->f_type = F_CONSOLE; else f->f_type = F_TTY; - (void)strcpy(f->f_un.f_fname, p + sizeof _PATH_DEV - 1); + (void)strlcpy(f->f_un.f_fname, p + sizeof(_PATH_DEV - 1), + sizeof(f->f_un.f_fname)); } else { - (void)strcpy(f->f_un.f_fname, p); + (void)strlcpy(f->f_un.f_fname, p, sizeof(f->f_un.f_fname)); f->f_type = F_FILE; } break; case '|': f->f_un.f_pipe.f_pid = 0; - (void)strcpy(f->f_un.f_pipe.f_pname, p + 1); + (void)strlcpy(f->f_un.f_fname, p + 1, sizeof(f->f_un.f_fname)); f->f_type = F_PIPE; break; @@ -1743,6 +1724,59 @@ decode(name, codetab) return (-1); } +void +markit(void) +{ + struct filed *f; + dq_t q; + + now = time((time_t *)NULL); + MarkSeq += TIMERINTVL; + if (MarkSeq >= MarkInterval) { + logmsg(LOG_INFO, "-- MARK --", + LocalHostName, ADDDATE|MARK); + MarkSeq = 0; + } + + for (f = Files; f; f = f->f_next) { + if (f->f_prevcount && now >= REPEATTIME(f)) { + dprintf("flush %s: repeated %d times, %d sec.\n", + TypeNames[f->f_type], f->f_prevcount, + repeatinterval[f->f_repeatcount]); + fprintlog(f, 0, (char *)NULL); + BACKOFF(f); + } + } + + /* Walk the dead queue, and see if we should signal somebody. */ + for (q = TAILQ_FIRST(&deadq_head); q != NULL; q = TAILQ_NEXT(q, dq_entries)) + switch (q->dq_timeout) { + case 0: + /* Already signalled once, try harder now. */ + if (kill(q->dq_pid, SIGKILL) != 0) + (void)deadq_remove(q->dq_pid); + break; + + case 1: + /* + * Timed out on dead queue, send terminate + * signal. Note that we leave the removal + * from the dead queue to reapchild(), which + * will also log the event (unless the process + * didn't even really exist, in case we simply + * drop it from the dead queue). + */ + if (kill(q->dq_pid, SIGTERM) != 0) + (void)deadq_remove(q->dq_pid); + /* FALLTHROUGH */ + + default: + q->dq_timeout--; + } + MarkSet = 0; + (void)alarm(TIMERINTVL); +} + /* * fork off and become a daemon, but wait for the child to come online * before returing to the parent, or we get disk thrashing at boot etc. @@ -1812,7 +1846,7 @@ timedout(sig) if (left == 0) errx(1, "timed out waiting for child"); else - exit(0); + _exit(0); } /* @@ -2006,7 +2040,7 @@ validate(sa, hname) /* traditional behaviour, allow everything */ return 1; - strlcpy(name, hname, sizeof name); + (void)strlcpy(name, hname, sizeof(name)); memset(&hints, 0, sizeof(hints)); hints.ai_family = PF_UNSPEC; hints.ai_socktype = SOCK_DGRAM;