]> Kevux Git Server - fll/commitdiff
Security: Fix memory leak in private_fl_directory_copy_recurse().
authorKevin Day <kevin@kevux.org>
Sun, 19 Mar 2023 22:03:55 +0000 (17:03 -0500)
committerKevin Day <kevin@kevux.org>
Sun, 19 Mar 2023 22:03:55 +0000 (17:03 -0500)
The cause is not as clear to me.
The memory address is being copied and so it should be being freed.
There could be some compiler optimization making a bad decision here.

Simplify the logic and remove extra checks.
Operate on the variable before replacing the pointer.
Memory leak seems to be gone now.

level_1/fl_directory/c/private-directory.c
level_3/fake/c/main/build.c

index 048705883262a2627e5d659cdd359e0202872d77..bc9220e9a15b2e9ce5dc652ee624a7de55e84b19 100644 (file)
@@ -8,23 +8,36 @@ extern "C" {
 #if !defined(_di_fl_directory_copy_)
   void private_fl_directory_copy_recurse(f_directory_recurse_t * const recurse) {
 
-    f_string_dynamics_t directories = f_string_dynamics_t_initialize;
     f_string_dynamics_t directories_original = f_string_dynamics_t_initialize;
 
     directories_original.array = recurse->listing.directory.array;
     directories_original.used = recurse->listing.directory.used;
     directories_original.size = recurse->listing.directory.size;
 
-    recurse->listing.directory.array = directories.array;
-    recurse->listing.directory.used = directories.used;
-    recurse->listing.directory.size = directories.size;
+    recurse->listing.directory.array = 0;
+    recurse->listing.directory.used = 0;
+    recurse->listing.directory.size = 0;
+
+    // Use a clean set for each recursion.
+    recurse->listing.block.used = 0;
+    recurse->listing.character.used = 0;
+    recurse->listing.directory.used = 0;
+    recurse->listing.regular.used = 0;
+    recurse->listing.link.used = 0;
+    recurse->listing.fifo.used = 0;
+    recurse->listing.socket.used = 0;
+    recurse->listing.unknown.used = 0;
 
     recurse->state.status = private_fl_directory_list(*recurse->source, 0, 0, recurse->flag & f_directory_recurse_flag_dereference_e, &recurse->listing);
 
     if (F_status_is_error(recurse->state.status)) {
 
       // Only the directory is to be freed because all others are preserved between recursions.
-      f_string_dynamics_resize(0, &directories);
+      f_string_dynamics_resize(0, &recurse->listing.directory);
+
+      recurse->listing.directory.array = directories_original.array;
+      recurse->listing.directory.used = directories_original.used;
+      recurse->listing.directory.size = directories_original.size;
 
       return;
     }
@@ -204,11 +217,12 @@ extern "C" {
       } // for
     }
 
+    // Only the directory is to be freed because all others are preserved between recursions.
+    f_string_dynamics_resize(0, &recurse->listing.directory);
+
     recurse->listing.directory.array = directories_original.array;
     recurse->listing.directory.used = directories_original.used;
     recurse->listing.directory.size = directories_original.size;
-
-    f_string_dynamics_resize(0, &directories);
   }
 #endif // !defined(_di_fl_directory_copy_)
 
index 16a8ce713e0f07111e9c81088a2d5973644f2b88..b52857f0e223419d179b8a3ca8fbbe5c7043d99c 100644 (file)
@@ -194,7 +194,7 @@ extern "C" {
 
     memcpy(path_source.string, source.string, sizeof(f_char_t) * source.used);
 
-    f_directory_recurse_t recurse = f_directory_recurse_t_initialize; // @fixme this is memory leaking on recurse.listing.
+    f_directory_recurse_t recurse = f_directory_recurse_t_initialize;
     recurse.verbose = &fake_print_verbose_copy;
     //recurse.failures = &failures; // @fixme this now needs to be handled by a callback in recurse (recurse.state.handle)., maybe make this a callback on f_directory_recurse_t?
     recurse.mode = mode;