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(
