create_temp_file/gen_path: prevent memory leak if gc == NULL

If gc == NULL, the data allocated in the alloc_gc_buf() call in
create_temp_file or the string_mod_const call in gen_path would never
be free'd.

These functions are currently never called that way, but let's prevent
future problems.

While touching create_temp_file, also remove the counter variable, which is
never read, simplify the do-while to a while loop, and truncate the prefix
(if needed) to preserve the random and extension of the created filename.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20171101220342.14648-5-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15703.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index 60a6cc7c044b66ca59591c4fcfd913d4b2ea812b..6d53cbfb00bf5da4ba9f026c697afd7ee7249081 100644 (file)
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -723,21 +723,26 @@ test_file(const char *filename)
 const char *
 create_temp_file(const char *directory, const char *prefix, struct gc_arena *gc)
 {
-    static unsigned int counter;
-    struct buffer fname = alloc_buf_gc(256, gc);
     int fd;
     const char *retfname = NULL;
     unsigned int attempts = 0;
+    char fname[256] = { 0 };
+    const char *fname_fmt = PACKAGE "_%.*s_%08lx%08lx.tmp";
+    const int max_prefix_len = sizeof(fname) - (sizeof(PACKAGE) + 7 + (2 * 8));
 
-    do
+    while (attempts < 6)
     {
         ++attempts;
-        ++counter;
 
-        buf_printf(&fname, PACKAGE "_%s_%08lx%08lx.tmp", prefix,
-                   (unsigned long) get_random(), (unsigned long) get_random());
+        if (!openvpn_snprintf(fname, sizeof(fname), fname_fmt, max_prefix_len,
+                              prefix, (unsigned long) get_random(),
+                              (unsigned long) get_random()))
+        {
+            msg(M_WARN, "ERROR: temporary filename too long");
+            return NULL;
+        }
 
-        retfname = gen_path(directory, BSTR(&fname), gc);
+        retfname = gen_path(directory, fname, gc);
         if (!retfname)
         {
             msg(M_WARN, "Failed to create temporary filename and path");
@@ -760,7 +765,6 @@ create_temp_file(const char *directory, const char *prefix, struct gc_arena *gc)
             return NULL;
         }
     }
-    while (attempts < 6);
 
     msg(M_WARN, "Failed to create temporary file after %i attempts", attempts);
     return NULL;
@@ -812,6 +816,12 @@ gen_path(const char *directory, const char *filename, struct gc_arena *gc)
 #else
     const int CC_PATH_RESERVED = CC_SLASH;
 #endif
+
+    if (!gc)
+    {
+        return NULL; /* Would leak memory otherwise */
+    }
+
     const char *safe_filename = string_mod_const(filename, CC_PRINT, CC_PATH_RESERVED, '_', gc);
 
     if (safe_filename