From cc79eb725f7b564269619ce4a64be2a80927d392 Mon Sep 17 00:00:00 2001 From: Jouni Malinen Date: Sat, 21 Mar 2020 12:57:37 +0200 Subject: [PATCH] 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 --- src/utils/common.c | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/src/utils/common.c b/src/utils/common.c index 27bf435d9..e5b3dcbd4 100644 --- a/src/utils/common.c +++ b/src/utils/common.c @@ -7,6 +7,7 @@ */ #include "includes.h" +#include #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);