[ntpsec commit] Forward-port fixes for NTP Classic [Bug 2845] Harden memory allocation in ntpd.

Eric S. Raymond esr at ntpsec.org
Thu Oct 22 14:46:17 UTC 2015


Module:    ntpsec
Branch:    master
Commit:    457363a28dcc8665e5ffa0fe13151c22bb11eb83
Changeset: http://git.ntpsec.org/ntpsec/commit/?id=457363a28dcc8665e5ffa0fe13151c22bb11eb83

Author:    Eric S. Raymond <esr at thyrsus.com>
Date:      Thu Oct 22 10:45:29 2015 -0400

Forward-port fixes for NTP Classic [Bug 2845] Harden memory allocation in ntpd.

---

 NEWS                    |  2 ++
 devel/TODO              |  3 +++
 include/ntp_stdlib.h    | 25 +++++++++++++++--------
 libntp/emalloc.c        | 53 +++++++++++++++++++++++++++++++++++++++++++++++++
 ntpd/ntp_config.c       |  7 ++++---
 ntpd/ntp_crypto.c       |  4 ++--
 ntpd/ntp_monitor.c      |  2 +-
 ntpdig/kod_management.c |  4 ++--
 ntpq/ntpq-subs.c        |  2 +-
 9 files changed, 85 insertions(+), 17 deletions(-)

diff --git a/NEWS b/NEWS
index e1a8e1a..3cd2af9 100644
--- a/NEWS
+++ b/NEWS
@@ -10,5 +10,7 @@ on user-visible changes.
 
 * [Bug 2836] DFC77 patches from Frank Kardel to make decoding more
   robust, and require 2 consecutive timestamps to be consistent.
+* [Bug 2845] Harden memory allocation in ntpd; implement and
+  use 'eallocarray(...)' where appropriate.
 
 // end
diff --git a/devel/TODO b/devel/TODO
index a5b0b51..ec586d2 100644
--- a/devel/TODO
+++ b/devel/TODO
@@ -143,6 +143,9 @@ Hal:
 > know things well enough to be sure that this sort of logic won't pick one 
 > back up.
 
+* Forward-port NTP Classic fix for [Bug 2830] just in case we want to
+  revive Autokey someday.
+
 == Old, sometime ancient stuff ==
 
 970711: Look Real Hard at changing the key stuff from u_long to u_int32.
diff --git a/include/ntp_stdlib.h b/include/ntp_stdlib.h
index 8d36c34..7f1ab4b 100644
--- a/include/ntp_stdlib.h
+++ b/include/ntp_stdlib.h
@@ -121,26 +121,35 @@ extern	uint32_t	addr2refid	(sockaddr_u *);
 /* emalloc.c */
 #ifndef EREALLOC_CALLSITE	/* ntp_malloc.h defines */
 extern	void *	ereallocz	(void *, size_t, size_t, int);
-#define	erealloczsite(p, n, o, z, f, l) ereallocz(p, n, o, (z))
-#define	emalloc(n)		ereallocz(NULL, n, 0, false)
+extern	void *	oreallocarray	(void *optr, size_t nmemb, size_t size);
+#define	erealloczsite(p, n, o, z, f, l) ereallocz((p), (n), (o), (z))
+#define	emalloc(n)		ereallocz(NULL, (n), 0, false)
 #define	emalloc_zero(c)		ereallocz(NULL, (c), 0, true)
-#define	erealloc(p, c)		ereallocz(p, (c), 0, false)
-#define erealloc_zero(p, n, o)	ereallocz(p, n, (o), true)
-extern	char *	estrdup_impl	(const char *);
+#define	erealloc(p, c)		ereallocz((p), (c), 0, false)
+#define erealloc_zero(p, n, o)	ereallocz((p), (n), (o), true)
+#define ereallocarray(p, n, s)	oreallocarray((p), (n), (s))
+#define eallocarray(n, s)	oreallocarray(NULL, (n), (s))
+extern	char *	estrdup_impl(const char *);
 #define	estrdup(s)		estrdup_impl(s)
 #else
 extern	void *	ereallocz	(void *, size_t, size_t, int,
 				 const char *, int);
+extern	void *	oreallocarray	(void *optr, size_t nmemb, size_t size,
+				 const char *, int);
 #define erealloczsite		ereallocz
 #define	emalloc(c)		ereallocz(NULL, (c), 0, false, \
 					  __FILE__, __LINE__)
 #define	emalloc_zero(c)		ereallocz(NULL, (c), 0, true, \
 					  __FILE__, __LINE__)
-#define	erealloc(p, c)		ereallocz(p, (c), 0, false, \
+#define	erealloc(p, c)		ereallocz((p), (c), 0, false,	\
 					  __FILE__, __LINE__)
-#define	erealloc_zero(p, n, o)	ereallocz(p, n, (o), true, \
+#define	erealloc_zero(p, n, o)	ereallocz((p), n, (o), true,	\
 					  __FILE__, __LINE__)
-extern	char *	estrdup_impl	(const char *, const char *, int);
+#define ereallocarray(p, n, s)	oreallocarray((p), (n), (s), \
+ 					  __FILE__, __LINE__)
+#define eallocarray(n, s)	oreallocarray(NULL, (n), (s), \
+ 					  __FILE__, __LINE__)
+extern	char *	estrdup_impl(const char *, const char *, int);
 #define	estrdup(s) estrdup_impl((s), __FILE__, __LINE__)
 #endif
 
diff --git a/libntp/emalloc.c b/libntp/emalloc.c
index 20ce63c..f98909d 100644
--- a/libntp/emalloc.c
+++ b/libntp/emalloc.c
@@ -60,6 +60,59 @@ ereallocz(
 	return mem;
 }
 
+/* oreallocarray.c is licensed under the following:
+ * Copyright (c) 2008 Otto Moerbeek <otto at drijf.net>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <stdint.h>
+
+/*
+ * This is sqrt(SIZE_MAX+1), as s1*s2 <= SIZE_MAX
+ * if both s1 < MUL_NO_OVERFLOW and s2 < MUL_NO_OVERFLOW
+ */
+#define MUL_NO_OVERFLOW	((size_t)1 << (sizeof(size_t) * 4))
+
+void *
+oreallocarray(
+	void *optr,
+	size_t nmemb,
+	size_t size
+#ifdef EREALLOC_CALLSITE		/* ntp_malloc.h */
+	,
+	const char *	file,
+	int		line
+#endif
+	)
+{
+	if ((nmemb >= MUL_NO_OVERFLOW || size >= MUL_NO_OVERFLOW) &&
+	    nmemb > 0 && SIZE_MAX / nmemb < size) {
+#ifndef EREALLOC_CALLSITE
+		msyslog(LOG_ERR, "fatal allocation size overflow");
+#else
+		msyslog(LOG_ERR,
+			"fatal allocation size overflow %s line %d",
+			file, line);
+#endif
+		exit(1);
+	}
+#ifndef EREALLOC_CALLSITE
+	return ereallocz(optr, (size * nmemb), 0, false);
+#else
+	return ereallocz(optr, (size * nmemb), 0, false, file, line);
+#endif
+}
 
 char *
 estrdup_impl(
diff --git a/ntpd/ntp_config.c b/ntpd/ntp_config.c
index 95a1f13..bc54565 100644
--- a/ntpd/ntp_config.c
+++ b/ntpd/ntp_config.c
@@ -4177,7 +4177,7 @@ config_sim(
 	serv_info = HEAD_PFIFO(sim_n->servers);
 	for (; serv_info != NULL; serv_info = serv_info->link)
 		simulation.num_of_servers++;
-	simulation.servers = emalloc(simulation.num_of_servers *
+	simulation.servers = eallocarray(simulation.num_of_servers,
 				     sizeof(simulation.servers[0]));
 
 	i = 0;
@@ -4723,8 +4723,9 @@ gettokens_netinfo (
 				if (namelist.ni_namelist_len == 0) continue;
 
 				config->val_list =
-				    emalloc(sizeof(char*) *
-				    (namelist.ni_namelist_len + 1));
+				    eallocarray(
+					(namelist.ni_namelist_len + 1),
+					sizeof(char*));
 				val_list = config->val_list;
 
 				for (index = 0;
diff --git a/ntpd/ntp_crypto.c b/ntpd/ntp_crypto.c
index 4a575d7..aba08cc 100644
--- a/ntpd/ntp_crypto.c
+++ b/ntpd/ntp_crypto.c
@@ -320,8 +320,8 @@ make_keylist(
 	 */
 	tstamp = crypto_time();
 	if (peer->keylist == NULL)
-		peer->keylist = emalloc(sizeof(keyid_t) *
-		    NTP_MAXSESSION);
+		peer->keylist = eallocarray(NTP_MAXSESSION,
+					    sizeof(keyid_t));
 
 	/*
 	 * Generate an initial key ID which is unique and greater than
diff --git a/ntpd/ntp_monitor.c b/ntpd/ntp_monitor.c
index 1ab1e88..2fcef93 100644
--- a/ntpd/ntp_monitor.c
+++ b/ntpd/ntp_monitor.c
@@ -180,7 +180,7 @@ mon_getmoremem(void)
 		      : mru_incalloc;
 
 	if (entries) {
-		chunk = emalloc(entries * sizeof(*chunk));
+		chunk = eallocarray(entries, sizeof(*chunk));
 		mru_alloc += entries;
 		for (chunk += entries; entries; entries--)
 			mon_free_entry(--chunk);
diff --git a/ntpdig/kod_management.c b/ntpdig/kod_management.c
index d75c978..64ee248 100644
--- a/ntpdig/kod_management.c
+++ b/ntpdig/kod_management.c
@@ -34,7 +34,7 @@ search_entry(
 		return 0;
 	}
 
-	*dst = emalloc(resc * sizeof(**dst));
+	*dst = eallocarray(resc, sizeof(**dst));
 
 	b = 0;
 	for (a = 0; a < kod_db_cnt; a++)
@@ -241,7 +241,7 @@ kod_init_kod_db(
 
 	rewind(db_s);
 
-	kod_db = emalloc(sizeof(kod_db[0]) * kod_db_cnt);
+	kod_db = eallocarray(kod_db_cnt, sizeof(kod_db[0]));
 
 	/* Read contents of file */
 	for (b = 0; 
diff --git a/ntpq/ntpq-subs.c b/ntpq/ntpq-subs.c
index d980ede..3eef1b1 100644
--- a/ntpq/ntpq-subs.c
+++ b/ntpq/ntpq-subs.c
@@ -2990,7 +2990,7 @@ mrulist(
 		goto cleanup_return;
 
 	/* construct an array of entry pointers in default order */
-	sorted = emalloc(mru_count * sizeof(*sorted));
+	sorted = eallocarray(mru_count, sizeof(*sorted));
 	ppentry = sorted;
 	if (MRUSORT_R_DEF != order) {
 		ITER_DLIST_BEGIN(mru_list, recent, mlink, mru)



More information about the vc mailing list