Check against integer overflow in int_array functions

int_array_concat() and int_array_add_unique() could potentially end up
overflowing the int type variable used to calculate their length. While
this is mostly theoretical for platforms that use 32-bit int, there
might be cases where a 16-bit int overflow could be hit. This could
result in accessing memory outside buffer bounds and potentially a
double free when realloc() ends up freeing the buffer.

All current uses of int_array_add_unique() and most uses of
int_array_concat() are currently limited by the buffer limits for the
local configuration parameter or frame length and as such, cannot hit
this overflow cases. The only case where a long enough int_array could
be generated is the combination of scan_freq values for a scan. The
memory and CPU resource needs for generating an int_array with 2^31
entries would not be realistic to hit in practice, but a device using
LP32 data model with 16-bit int could hit this case.

It is better to have more robust checks even if this could not be
reached in practice, so handle cases where more than INT_MAX entries
would be added to an int_array as memory allocation failures instead of
allowing the overflow case to proceed.

Signed-off-by: Jouni Malinen <j@w1.fi>

diff --git a/src/utils/common.c b/src/utils/common.c
index 27bf435d96910f56cedf505d88b5aa3f8fb7ba63..e5b3dcbd4909498acc5047f0f436f8a17ea56516 100644 (file)
--- a/src/utils/common.c
+++ b/src/utils/common.c
@@ -7,6 +7,7 @@
  */
 
 #include "includes.h"
+#include <limits.h>
 
 #include "common/ieee802_11_defs.h"
 #include "common.h"
@@ -885,18 +886,28 @@ int int_array_len(const int *a)
 
 void int_array_concat(int **res, const int *a)
 {
-       int reslen, alen, i;
+       int reslen, alen, i, new_len;
        int *n;
 
        reslen = int_array_len(*res);
        alen = int_array_len(a);
-
-       n = os_realloc_array(*res, reslen + alen + 1, sizeof(int));
-       if (n == NULL) {
+       new_len = reslen + alen + 1;
+       if (reslen < 0 || alen < 0 || new_len < 0) {
+               /* This should not really happen, but if it did, something
+                * overflowed. Do not try to merge the arrays; instead, make
+                * this behave like memory allocation failure to avoid messing
+                * up memory. */
                os_free(*res);
                *res = NULL;
                return;
        }
+       n = os_realloc_array(*res, new_len, sizeof(int));
+       if (n == NULL) {
+               if (new_len)
+                       os_free(*res);
+               *res = NULL;
+               return;
+       }
        for (i = 0; i <= alen; i++)
                n[reslen + i] = a[i];
        *res = n;
@@ -952,6 +963,15 @@ void int_array_add_unique(int **res, int a)
                        return; /* already in the list */
        }
 
+       if (reslen > INT_MAX - 2) {
+               /* This should not really happen in practice, but if it did,
+                * something would overflow. Do not try to add the new value;
+                * instead, make this behave like memory allocation failure to
+                * avoid messing up memory. */
+               os_free(*res);
+               *res = NULL;
+               return;
+       }
        n = os_realloc_array(*res, reslen + 2, sizeof(int));
        if (n == NULL) {
                os_free(*res);