From 164e8e66269efccf8d9b836dbb83646137c3858c Mon Sep 17 00:00:00 2001 From: Kevin Day Date: Fri, 7 Jun 2024 00:22:47 -0500 Subject: [PATCH] Security: Incorrect sizeof() used in process pointers of the Controller program. The "processs" structure is an array of pointers to Controller processes. This gets rather confusing in that what the pointers are pointing to must be allocated (and deallocated) as well. The allocation and de-allocaton process is consistent and doesn't memory leak. However, both of these are using a sizeof() with a pointer type rather than the raw type for the inner value. The allocation logic on the outer array is using the raw type when a pointer should be used. Essentially, I accidentally reversed the allocation logic. I am amazed that this worked for so long without getting noticed. Building the Controller program as a stand alone program somehow revealed this bug. This has the added bonus of allocating less memory. For example, using a test run without any entry file to execute: Before this change: total heap usage: 54 allocs, 54 frees, 46,622 bytes allocated After this change: total heap usage: 54 allocs, 54 frees, 26,751 bytes allocated This appears to, in part, be a regression of commit c85e0e6892da72b0737fde2cd8302b0505a9b12f. --- level_3/controller/c/common/private-process.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/level_3/controller/c/common/private-process.c b/level_3/controller/c/common/private-process.c index 9d9d742..048cd37 100644 --- a/level_3/controller/c/common/private-process.c +++ b/level_3/controller/c/common/private-process.c @@ -106,11 +106,11 @@ extern "C" { if (processs->array[i]) { controller_process_delete_simple(processs->array[i]); - f_memory_delete(1, sizeof(controller_process_t *), (void **) & processs->array[i]); + f_memory_delete(1, sizeof(controller_process_t), (void **) &processs->array[i]); } } // for - status = f_memory_resize(processs->size, length, sizeof(controller_process_t), (void **) & processs->array); + status = f_memory_resize(processs->size, length, sizeof(controller_process_t *), (void **) &processs->array); if (F_status_is_error(status)) return status; if (length) { @@ -119,7 +119,7 @@ extern "C" { // The lock must be initialized, but only once, so initialize immediately upon allocation. for (; processs->size < length; ++processs->size) { - status = f_memory_new(1, sizeof(controller_process_t *), (void **) &processs->array[processs->size]); + status = f_memory_new(1, sizeof(controller_process_t), (void **) &processs->array[processs->size]); if (F_status_is_error_not(status)) { process = processs->array[processs->size]; -- 1.8.3.1