Harden create_temp_filename() (version 2)

By hardening the create_temp_filename() function to check if the generated
filename exists and to create the temp file with only S_IRUSR|S_IWUSR bit
files set before calling the script, it should become even more difficult to
exploit such a scenario.

After a discussion on the mailing list, Fabian Knittel provided an enhanced
version of the inital patch which is added to this patch.

This patch also renames create_temp_filename() to create_temp_file(), as this
patch also creates the temporary file.  The function returns the filename of the
created file, or NULL on error.

Signed-off-by: David Sommerseth <dazo@users.sourceforge.net>
Signed-off-by: Fabian Knittel <fabian.knittel@avona.com>
Acked-by: Gert Doering <gert@greenie.muc.de>

diff --git a/misc.c b/misc.c
index 507bcc2ca17d9a93ccef1288f6d193f40afe496d..0ceeb2d09dec335a93f3fe1e8c361ba2525407b9 100644 (file)
--- a/misc.c
+++ b/misc.c
@@ -1165,25 +1165,57 @@ test_file (const char *filename)
 
 /* create a temporary filename in directory */
 const char *
-create_temp_filename (const char *directory, const char *prefix, struct gc_arena *gc)
+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;
 
-  mutex_lock_static (L_CREATE_TEMP);
-  ++counter;
-  mutex_unlock_static (L_CREATE_TEMP);
-
-  {
-    uint8_t rndbytes[16];
-    const char *rndstr;
-
-    prng_bytes (rndbytes, sizeof (rndbytes));
-    rndstr = format_hex_ex (rndbytes, sizeof (rndbytes), 40, 0, NULL, gc);
-    buf_printf (&fname, PACKAGE "_%s_%s.tmp", prefix, rndstr);
-  }
+  do
+    {
+      uint8_t rndbytes[16];
+      const char *rndstr;
+
+      ++attempts;
+      mutex_lock_static (L_CREATE_TEMP);
+      ++counter;
+      mutex_unlock_static (L_CREATE_TEMP);
+
+      prng_bytes (rndbytes, sizeof rndbytes);
+      rndstr = format_hex_ex (rndbytes, sizeof rndbytes, 40, 0, NULL, gc);
+      buf_printf (&fname, PACKAGE "_%s_%s.tmp", prefix, rndstr);
+
+      retfname = gen_path (directory, BSTR (&fname), gc);
+      if (!retfname)
+        {
+          msg (M_FATAL, "Failed to create temporary filename and path");
+          return NULL;
+        }
+
+      /* Atomically create the file.  Errors out if the file already
+         exists.  */
+      fd = open (retfname, O_CREAT | O_EXCL | O_WRONLY, S_IRUSR | S_IWUSR);
+      if (fd != -1)
+        {
+          close (fd);
+          return retfname;
+        }
+      else if (fd == -1 && errno != EEXIST)
+        {
+          /* Something else went wrong, no need to retry.  */
+          struct gc_arena gcerr = gc_new ();
+          msg (M_FATAL, "Could not create temporary file '%s': %s",
+               retfname, strerror_ts (errno, &gcerr));
+          gc_free (&gcerr);
+          return NULL;
+        }
+    }
+  while (attempts < 6);
 
-  return gen_path (directory, BSTR (&fname), gc);
+  msg (M_FATAL, "Failed to create temporary file after %i attempts", attempts);
+  return NULL;
 }
 
 /*

diff --git a/misc.h b/misc.h
index 328107dedf90c0f446bb38a88b5d306878fd91ff..d5ad7747d97e4fe5768d2dc49a3b3adc54baa922 100644 (file)
--- a/misc.h
+++ b/misc.h
@@ -218,8 +218,8 @@ long int get_random(void);
 /* return true if filename can be opened for read */
 bool test_file (const char *filename);
 
-/* create a temporary filename in directory */
-const char *create_temp_filename (const char *directory, const char *prefix, struct gc_arena *gc);
+/* create a temporary file in directory, returns the filename of the created file */
+const char *create_temp_file (const char *directory, const char *prefix, struct gc_arena *gc);
 
 /* put a directory and filename together */
 const char *gen_path (const char *directory, const char *filename, struct gc_arena *gc);