Hi, our syslogd is also vulnerable to rsyslog's CVE-2014-3634. The CVE is about parsing the priority from network clients. The priority boundary isn't properly checked, which could lead to out of bounds access later on.
sysklogd's commit message is pretty extensive, so have a read here: http://git.infodrom.org/?p=infodrom/sysklogd;a=commitdiff;h=5b156a903326e7d1403c1750f3721b646eaf551c The sysklogd patch (and mine which is based on it) have a change in behavior. If the priority chunk of the string is invalid, the whole line will be logged. Previously, it would log the line somewhere after the initial '<' char, which initiates the priority parsing. My proposed diff should be simpler by being less intrusive. From my point of view, there is no need to work with strlen() and adding new variables. The sysklogd fix only handles network code. Same algorithm is used while reading from /dev/klog... Although I doubt that it's a practical attack vector, let's fix it for the sake of completeness. Thoughts? Okays? Tobias Index: syslogd.c =================================================================== RCS file: /cvs/src/usr.sbin/syslogd/syslogd.c,v retrieving revision 1.129 diff -u -p -r1.129 syslogd.c --- syslogd.c 6 Oct 2014 19:36:34 -0000 1.129 +++ syslogd.c 12 Oct 2014 11:03:51 -0000 @@ -104,6 +104,7 @@ const char ctty[] = _PATH_CONSOLE; #define MAXUNAMES 20 /* maximum number of user names */ +#define MAX_PRI 191 /* maximum priority per RFC 3164 */ /* * Flags to logmsg(). @@ -684,12 +685,16 @@ printline(char *hname, char *msg) /* test for special codes */ pri = DEFUPRI; p = msg; - if (*p == '<') { + if (p[0] == '<' && p[1] != '>') { pri = 0; - while (isdigit((unsigned char)*++p)) + while (isdigit((unsigned char)*++p) && pri <= MAX_PRI) pri = 10 * pri + (*p - '0'); - if (*p == '>') + if (*p == '>' && pri <= MAX_PRI) ++p; + else { + pri = DEFUPRI; + p = msg; + } } if (pri &~ (LOG_FACMASK|LOG_PRIMASK)) pri = DEFUPRI; @@ -720,19 +725,22 @@ void printsys(char *msg) { int c, pri, flags; - char *lp, *p, *q, line[MAXLINE + 1]; + char *lp, *p, *q, *r, line[MAXLINE + 1]; (void)snprintf(line, sizeof line, "%s: ", _PATH_UNIX); lp = line + strlen(line); for (p = msg; *p != '\0'; ) { flags = SYNC_FILE | ADDDATE; /* fsync file after write */ pri = DEFSPRI; - if (*p == '<') { + r = p; + if (r[0] == '<' && r[1] != '>') { pri = 0; - while (isdigit((unsigned char)*++p)) - pri = 10 * pri + (*p - '0'); - if (*p == '>') - ++p; + while (isdigit((unsigned char)*++r) && pri <= MAX_PRI) + pri = 10 * pri + (*r - '0'); + if (*r == '>' && pri <= MAX_PRI) + p = ++r; + else + pri = DEFSPRI; } else { /* kernel printf's come out on console */ flags |= IGN_CONS;
