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>
This commit is contained in:
Jouni Malinen 2020-03-21 12:57:37 +02:00
parent 2af3d99cd3
commit cc79eb725f

View file

@ -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);