From 7ffee735247353c6af7871d41dd064ffb80b064a Mon Sep 17 00:00:00 2001
From: Miroslav Lichvar <mlichvar@redhat.com>
Date: Thu, 24 Aug 2017 12:10:46 +0200
Subject: [PATCH] memory: check for overflow when (re)allocating array

When (re)allocating an array with very large number of elements using
the MallocArray or ReallocArray macros, the calculated size of the array
could overflow size_t and less memory would be allocated than requested.

Add new functions for (re)allocating arrays that check the size and use
them in the MallocArray and ReallocArray macros.

This couldn't be exploited, because all arrays that can grow with cmdmon
or NTP requests already have their size checked before allocation, or
they are much smaller than memory allocated for structures to which they
are related (i.e. ntp_core and sourcestats instances), so a memory
allocation would fail before their size could overflow.

This issue was found in an audit performed by Cure53 and sponsored by
Mozilla.
---
 array.c  |  7 +------
 memory.c | 26 ++++++++++++++++++++++++++
 memory.h |  6 ++++--
 3 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/array.c b/array.c
index ffe5a4e..d70cff9 100644
--- a/array.c
+++ b/array.c
@@ -66,8 +66,6 @@ ARR_DestroyInstance(ARR_Instance array)
 static void
 realloc_array(ARR_Instance array, unsigned int min_size)
 {
-  size_t data_size;
-
   assert(min_size <= 2 * min_size);
   if (array->allocated >= min_size && array->allocated <= 2 * min_size)
     return;
@@ -79,10 +77,7 @@ realloc_array(ARR_Instance array, unsigned int min_size)
     array->allocated = min_size;
   }
 
-  data_size = (size_t)array->elem_size * array->allocated;
-  assert(data_size / array->elem_size == array->allocated);
-
-  array->data = Realloc(array->data, data_size);
+  array->data = Realloc2(array->data, array->allocated, array->elem_size);
 }
 
 void *
diff --git a/memory.c b/memory.c
index 2ce6ff3..b5c356c 100644
--- a/memory.c
+++ b/memory.c
@@ -54,6 +54,32 @@ Realloc(void *ptr, size_t size)
   return r;
 }
 
+static size_t
+get_array_size(size_t nmemb, size_t size)
+{
+  size_t array_size;
+
+  array_size = nmemb * size;
+
+  /* Check for overflow */
+  if (nmemb > 0 && array_size / nmemb != size)
+    LOG_FATAL("Could not allocate memory");
+
+  return array_size;
+}
+
+void *
+Malloc2(size_t nmemb, size_t size)
+{
+  return Malloc(get_array_size(nmemb, size));
+}
+
+void *
+Realloc2(void *ptr, size_t nmemb, size_t size)
+{
+  return Realloc(ptr, get_array_size(nmemb, size));
+}
+
 char *
 Strdup(const char *s)
 {
diff --git a/memory.h b/memory.h
index 3ec0f04..110cf6a 100644
--- a/memory.h
+++ b/memory.h
@@ -30,12 +30,14 @@
 /* Wrappers checking for errors */
 extern void *Malloc(size_t size);
 extern void *Realloc(void *ptr, size_t size);
+extern void *Malloc2(size_t nmemb, size_t size);
+extern void *Realloc2(void *ptr, size_t nmemb, size_t size);
 extern char *Strdup(const char *s);
 
 /* Convenient macros */
 #define MallocNew(T) ((T *) Malloc(sizeof(T)))
-#define MallocArray(T, n) ((T *) Malloc((n) * sizeof(T)))
-#define ReallocArray(T,n,x) ((T *) Realloc((void *)(x), (n)*sizeof(T)))
+#define MallocArray(T, n) ((T *) Malloc2(n, sizeof(T)))
+#define ReallocArray(T, n, x) ((T *) Realloc2((void *)(x), n, sizeof(T)))
 #define Free(x) free(x)
 
 #endif /* GOT_MEMORY_H */