]> Kevux Git Server - fll/commitdiff
Security: Incorrect sizeof() used in process pointers of the Controller program.
authorKevin Day <Kevin@kevux.org>
Fri, 7 Jun 2024 05:22:47 +0000 (00:22 -0500)
committerKevin Day <Kevin@kevux.org>
Fri, 7 Jun 2024 05:34:38 +0000 (00:34 -0500)
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

index 9d9d7427c94974f873558e4fac8b9a8427d389b8..048cd372c27d431cc83c52f7c63ec33fe8f58042 100644 (file)
@@ -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];