From: Kevin Day Date: Fri, 7 Jun 2024 05:22:47 +0000 (-0500) Subject: Security: Incorrect sizeof() used in process pointers of the Controller program. X-Git-Tag: 0.6.11~50 X-Git-Url: https://git.kevux.org/?a=commitdiff_plain;h=164e8e66269efccf8d9b836dbb83646137c3858c;p=fll 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. --- 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];