From: Tobias Stoeckmann <tobias@stoeckmann.org>
Date: Tue, 13 Aug 2024 17:44:48 +0000 (+0200)
Subject: shared: Add defensive measures to array handling
X-Git-Tag: v34~512
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f4b858c704cd376fe338cbdc7d5809d3270b2691;p=thirdparty%2Fkmod.git

shared: Add defensive measures to array handling

- Make sure that SIZE_MAX boundaries are never crossed
- Clear pointer address in struct during array_free_array
- Do nothing if array_pop is called with empty array

Also added test case for pop behavior and extended tests with
more checks.

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com>
Link: https://github.com/kmod-project/kmod/pull/68
Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
---

diff --git a/shared/array.c b/shared/array.c
index 8f0dd0d4..ee776a3a 100644
--- a/shared/array.c
+++ b/shared/array.c
@@ -5,6 +5,7 @@
 
 #include <assert.h>
 #include <errno.h>
+#include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
 
@@ -15,7 +16,11 @@
 
 static int array_realloc(struct array *array, size_t new_total)
 {
-	void *tmp = realloc(array->array, sizeof(void *) * new_total);
+	void *tmp;
+
+	if (SIZE_MAX / sizeof(void *) < new_total)
+		return -ENOMEM;
+	tmp = realloc(array->array, sizeof(void *) * new_total);
 	if (tmp == NULL)
 		return -ENOMEM;
 	array->array = tmp;
@@ -37,7 +42,11 @@ int array_append(struct array *array, const void *element)
 	size_t idx;
 
 	if (array->count + 1 >= array->total) {
-		int r = array_realloc(array, array->total + array->step);
+		int r;
+
+		if (SIZE_MAX - array->total < array->step)
+			return -ENOMEM;
+		r = array_realloc(array, array->total + array->step);
 		if (r < 0)
 			return r;
 	}
@@ -58,6 +67,8 @@ int array_append_unique(struct array *array, const void *element)
 }
 
 void array_pop(struct array *array) {
+	if (array->count == 0)
+		return;
 	array->count--;
 	if (array->count + array->step < array->total) {
 		int r = array_realloc(array, array->total - array->step);
@@ -68,6 +79,7 @@ void array_pop(struct array *array) {
 
 void array_free_array(struct array *array) {
 	free(array->array);
+	array->array = NULL;
 	array->count = 0;
 	array->total = 0;
 }
diff --git a/testsuite/test-array.c b/testsuite/test-array.c
index 61350f86..838dcec9 100644
--- a/testsuite/test-array.c
+++ b/testsuite/test-array.c
@@ -130,13 +130,16 @@ static int test_array_remove_at(const struct test *t)
 	assert_return(array.count == 2, EXIT_FAILURE);
 	assert_return(array.array[0] == c1, EXIT_FAILURE);
 	assert_return(array.array[1] == c2, EXIT_FAILURE);
+	assert_return(array.total == 4, EXIT_FAILURE);
 
 	array_remove_at(&array, 0);
 	assert_return(array.count == 1, EXIT_FAILURE);
 	assert_return(array.array[0] == c2, EXIT_FAILURE);
+	assert_return(array.total == 2, EXIT_FAILURE);
 
 	array_remove_at(&array, 0);
 	assert_return(array.count == 0, EXIT_FAILURE);
+	assert_return(array.total == 2, EXIT_FAILURE);
 
 	array_append(&array, c1);
 	array_append(&array, c2);
@@ -146,6 +149,7 @@ static int test_array_remove_at(const struct test *t)
 	assert_return(array.count == 2, EXIT_FAILURE);
 	assert_return(array.array[0] == c1, EXIT_FAILURE);
 	assert_return(array.array[1] == c3, EXIT_FAILURE);
+	assert_return(array.total == 4, EXIT_FAILURE);
 
 	array_free_array(&array);
 
@@ -178,6 +182,10 @@ static int test_array_pop(const struct test *t)
 
 	assert_return(array.count == 0, EXIT_FAILURE);
 
+	array_pop(&array);
+
+	assert_return(array.count == 0, EXIT_FAILURE);
+
 	array_free_array(&array);
 
 	return 0;