backport of: From bb928ef08eec020ef6008f3a140702ccc0536b8e Mon Sep 17 00:00:00 2001 From: <jnperlin@hydra.localnet> Date: Sat, 3 Oct 2015 09:08:20 +0200 Subject: [PATCH] [TALOS-CAN-0055] Infinite loop if extended logging enabled and the logfile and keyfile are the same --- ChangeLog | 2 ++ include/ntp_stdlib.h | 1 + include/ntp_syslog.h | 1 + libntp/authreadkeys.c | 89 ++++++++++++++++++++++++++++++++++++++++----------- libntp/msyslog.c | 12 +++++++ 5 files changed, 87 insertions(+), 18 deletions(-) Index: ntp-4.2.6.p5+dfsg/include/ntp_stdlib.h =================================================================== --- ntp-4.2.6.p5+dfsg.orig/include/ntp_stdlib.h 2015-10-23 08:30:31.515986332 -0400 +++ ntp-4.2.6.p5+dfsg/include/ntp_stdlib.h 2015-10-23 08:30:31.511986291 -0400 @@ -46,7 +46,8 @@ __attribute__((__format__(__printf__, 3, 4))); extern void msyslog(int, const char *, ...) __attribute__((__format__(__printf__, 2, 3))); - +extern void mvsyslog(int, const char *, va_list) + __attribute__((__format__(__printf__, 2, 0))); /* * When building without OpenSSL, use a few macros of theirs to * minimize source differences in NTP. Index: ntp-4.2.6.p5+dfsg/include/ntp_syslog.h =================================================================== --- ntp-4.2.6.p5+dfsg.orig/include/ntp_syslog.h 2015-10-23 08:30:31.515986332 -0400 +++ ntp-4.2.6.p5+dfsg/include/ntp_syslog.h 2015-10-23 08:30:31.511986291 -0400 @@ -9,6 +9,7 @@ # ifdef VMS extern void msyslog(); +extern void mvsyslog(); # else # ifndef SYS_VXWORKS # include <syslog.h> Index: ntp-4.2.6.p5+dfsg/libntp/authreadkeys.c =================================================================== --- ntp-4.2.6.p5+dfsg.orig/libntp/authreadkeys.c 2015-10-23 08:30:31.515986332 -0400 +++ ntp-4.2.6.p5+dfsg/libntp/authreadkeys.c 2015-10-23 08:32:05.104935695 -0400 @@ -61,6 +61,40 @@ } +/* TALOS-CAN-0055: possibly DoS attack by setting the key file to the + * log file. This is hard to prevent (it would need to check two files + * to be the same on the inode level, which will not work so easily with + * Windows or VMS) but we can avoid the self-amplification loop: We only + * log the first 5 errors, silently ignore the next 10 errors, and give + * up when when we have found more than 15 errors. + * + * This avoids the endless file iteration we will end up with otherwise, + * and also avoids overflowing the log file. + * + * Nevertheless, once this happens, the keys are gone since this would + * require a save/swap strategy that is not easy to apply due to the + * data on global/static level. + */ + +static const size_t nerr_loglimit = 5u; +static const size_t nerr_maxlimit = 15; + +static void log_maybe(size_t*, const char*, ...) __attribute__((__format__(__printf__, 2, 3))); + +static void +log_maybe( + size_t *pnerr, + const char *fmt , + ...) +{ + va_list ap; + if (++(*pnerr) <= nerr_loglimit) { + va_start(ap, fmt); + mvsyslog(LOG_ERR, fmt, ap); + va_end(ap); + } +} + /* * authreadkeys - (re)read keys from a file. */ @@ -78,7 +112,7 @@ u_char keystr[20]; int len; int j; - + size_t nerr; /* * Open file. Complain and return if it can't be opened. */ @@ -98,7 +132,10 @@ /* * Now read lines from the file, looking for key entries */ + nerr = 0; while ((line = fgets(buf, sizeof buf, fp)) != NULL) { + if (nerr > nerr_maxlimit) + break; token = nexttok(&line); if (token == NULL) continue; @@ -108,15 +145,16 @@ */ keyno = atoi(token); if (keyno == 0) { - msyslog(LOG_ERR, - "authreadkeys: cannot change key %s", token); + log_maybe(&nerr, + "authreadkeys: cannot change key %s", + token); continue; } if (keyno > NTP_MAXKEY) { - msyslog(LOG_ERR, - "authreadkeys: key %s > %d reserved for Autokey", - token, NTP_MAXKEY); + log_maybe(&nerr, + "authreadkeys: key %s > %d reserved for Autokey", + token, NTP_MAXKEY); continue; } @@ -125,8 +163,9 @@ */ token = nexttok(&line); if (token == NULL) { - msyslog(LOG_ERR, - "authreadkeys: no key type for key %d", keyno); + log_maybe(&nerr, + "authreadkeys: no key type for key %d", + keyno); continue; } #ifdef OPENSSL @@ -138,13 +177,15 @@ */ keytype = keytype_from_text(token, NULL); if (keytype == 0) { - msyslog(LOG_ERR, - "authreadkeys: invalid type for key %d", keyno); + log_maybe(&nerr, + "authreadkeys: invalid type for key %d", + keyno); continue; } if (EVP_get_digestbynid(keytype) == NULL) { - msyslog(LOG_ERR, - "authreadkeys: no algorithm for key %d", keyno); + log_maybe(&nerr, + "authreadkeys: no algorithm for key %d", + keyno); continue; } #else /* OPENSSL */ @@ -154,8 +195,9 @@ * 'm' for compatibility. */ if (!(*token == 'M' || *token == 'm')) { - msyslog(LOG_ERR, - "authreadkeys: invalid type for key %d", keyno); + log_maybe(&nerr, + "authreadkeys: invalid type for key %d", + keyno); continue; } keytype = KEY_TYPE_MD5; @@ -169,8 +211,8 @@ */ token = nexttok(&line); if (token == NULL) { - msyslog(LOG_ERR, - "authreadkeys: no key for key %d", keyno); + log_maybe(&nerr, + "authreadkeys: no key for key %d", keyno); continue; } len = strlen(token); @@ -186,8 +228,9 @@ for (j = 0; j < jlim; j++) { ptr = strchr(hex, tolower(token[j])); if (ptr == NULL) { - msyslog(LOG_ERR, - "authreadkeys: invalid hex digit for key %d", keyno); + log_maybe(&nerr, + "authreadkeys: invalid hex digit for key %d", + keyno); continue; } temp = (u_char)(ptr - hex); @@ -200,5 +243,15 @@ } } fclose(fp); + if (nerr > nerr_maxlimit) { + msyslog(LOG_ERR, + "authreadkeys: emergency break after %u errors", + nerr); + return (0); + } else if (nerr > nerr_loglimit) { + msyslog(LOG_ERR, + "authreadkeys: found %u more error(s)", + nerr - nerr_loglimit); + } return (1); } Index: ntp-4.2.6.p5+dfsg/libntp/msyslog.c =================================================================== --- ntp-4.2.6.p5+dfsg.orig/libntp/msyslog.c 2015-10-23 08:30:31.515986332 -0400 +++ ntp-4.2.6.p5+dfsg/libntp/msyslog.c 2015-10-23 08:30:31.511986291 -0400 @@ -271,6 +271,18 @@ return rc; } +void +mvsyslog( + int level, + const char * fmt, + va_list ap + ) +{ + char buf[1024]; + mvsnprintf(buf, sizeof(buf), fmt, ap); + addto_syslog(level, buf); +} + void msyslog(