From: Kevin Day <kevin@kevux.org>
Date: Sun, 19 Mar 2023 22:03:55 +0000 (-0500)
Subject: Security: Fix memory leak in private_fl_directory_copy_recurse().
X-Git-Url: https://git.kevux.org/?a=commitdiff_plain;h=a6720d54d4315590e70884c96d48ec8e098ae28c;p=fll

Security: Fix memory leak in private_fl_directory_copy_recurse().

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.
---

diff --git a/level_1/fl_directory/c/private-directory.c b/level_1/fl_directory/c/private-directory.c
index 0487058..bc9220e 100644
--- a/level_1/fl_directory/c/private-directory.c
+++ b/level_1/fl_directory/c/private-directory.c
@@ -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_)
 
diff --git a/level_3/fake/c/main/build.c b/level_3/fake/c/main/build.c
index 16a8ce7..b52857f 100644
--- a/level_3/fake/c/main/build.c
+++ b/level_3/fake/c/main/build.c
@@ -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;