Browse code

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>

Steffan Karger authored on 2017/11/02 07:03:42
Showing 1 changed files
... ...
@@ -723,21 +723,26 @@ test_file(const char *filename)
723 723
 const char *
724 724
 create_temp_file(const char *directory, const char *prefix, struct gc_arena *gc)
725 725
 {
726
-    static unsigned int counter;
727
-    struct buffer fname = alloc_buf_gc(256, gc);
728 726
     int fd;
729 727
     const char *retfname = NULL;
730 728
     unsigned int attempts = 0;
729
+    char fname[256] = { 0 };
730
+    const char *fname_fmt = PACKAGE "_%.*s_%08lx%08lx.tmp";
731
+    const int max_prefix_len = sizeof(fname) - (sizeof(PACKAGE) + 7 + (2 * 8));
731 732
 
732
-    do
733
+    while (attempts < 6)
733 734
     {
734 735
         ++attempts;
735
-        ++counter;
736 736
 
737
-        buf_printf(&fname, PACKAGE "_%s_%08lx%08lx.tmp", prefix,
738
-                   (unsigned long) get_random(), (unsigned long) get_random());
737
+        if (!openvpn_snprintf(fname, sizeof(fname), fname_fmt, max_prefix_len,
738
+                              prefix, (unsigned long) get_random(),
739
+                              (unsigned long) get_random()))
740
+        {
741
+            msg(M_WARN, "ERROR: temporary filename too long");
742
+            return NULL;
743
+        }
739 744
 
740
-        retfname = gen_path(directory, BSTR(&fname), gc);
745
+        retfname = gen_path(directory, fname, gc);
741 746
         if (!retfname)
742 747
         {
743 748
             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)
760 760
             return NULL;
761 761
         }
762 762
     }
763
-    while (attempts < 6);
764 763
 
765 764
     msg(M_WARN, "Failed to create temporary file after %i attempts", attempts);
766 765
     return NULL;
... ...
@@ -812,6 +816,12 @@ gen_path(const char *directory, const char *filename, struct gc_arena *gc)
812 812
 #else
813 813
     const int CC_PATH_RESERVED = CC_SLASH;
814 814
 #endif
815
+
816
+    if (!gc)
817
+    {
818
+        return NULL; /* Would leak memory otherwise */
819
+    }
820
+
815 821
     const char *safe_filename = string_mod_const(filename, CC_PRINT, CC_PATH_RESERVED, '_', gc);
816 822
 
817 823
     if (safe_filename