Browse code

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>

David Sommerseth authored on 2010/04/17 05:02:36
Showing 2 changed files
... ...
@@ -1165,25 +1165,57 @@ test_file (const char *filename)
1165 1165
 
1166 1166
 /* create a temporary filename in directory */
1167 1167
 const char *
1168
-create_temp_filename (const char *directory, const char *prefix, struct gc_arena *gc)
1168
+create_temp_file (const char *directory, const char *prefix, struct gc_arena *gc)
1169 1169
 {
1170 1170
   static unsigned int counter;
1171 1171
   struct buffer fname = alloc_buf_gc (256, gc);
1172
+  int fd;
1173
+  const char *retfname = NULL;
1174
+  unsigned int attempts = 0;
1172 1175
 
1173
-  mutex_lock_static (L_CREATE_TEMP);
1174
-  ++counter;
1175
-  mutex_unlock_static (L_CREATE_TEMP);
1176
-
1177
-  {
1178
-    uint8_t rndbytes[16];
1179
-    const char *rndstr;
1180
-
1181
-    prng_bytes (rndbytes, sizeof (rndbytes));
1182
-    rndstr = format_hex_ex (rndbytes, sizeof (rndbytes), 40, 0, NULL, gc);
1183
-    buf_printf (&fname, PACKAGE "_%s_%s.tmp", prefix, rndstr);
1184
-  }
1176
+  do
1177
+    {
1178
+      uint8_t rndbytes[16];
1179
+      const char *rndstr;
1180
+
1181
+      ++attempts;
1182
+      mutex_lock_static (L_CREATE_TEMP);
1183
+      ++counter;
1184
+      mutex_unlock_static (L_CREATE_TEMP);
1185
+
1186
+      prng_bytes (rndbytes, sizeof rndbytes);
1187
+      rndstr = format_hex_ex (rndbytes, sizeof rndbytes, 40, 0, NULL, gc);
1188
+      buf_printf (&fname, PACKAGE "_%s_%s.tmp", prefix, rndstr);
1189
+
1190
+      retfname = gen_path (directory, BSTR (&fname), gc);
1191
+      if (!retfname)
1192
+        {
1193
+          msg (M_FATAL, "Failed to create temporary filename and path");
1194
+          return NULL;
1195
+        }
1196
+
1197
+      /* Atomically create the file.  Errors out if the file already
1198
+         exists.  */
1199
+      fd = open (retfname, O_CREAT | O_EXCL | O_WRONLY, S_IRUSR | S_IWUSR);
1200
+      if (fd != -1)
1201
+        {
1202
+          close (fd);
1203
+          return retfname;
1204
+        }
1205
+      else if (fd == -1 && errno != EEXIST)
1206
+        {
1207
+          /* Something else went wrong, no need to retry.  */
1208
+          struct gc_arena gcerr = gc_new ();
1209
+          msg (M_FATAL, "Could not create temporary file '%s': %s",
1210
+               retfname, strerror_ts (errno, &gcerr));
1211
+          gc_free (&gcerr);
1212
+          return NULL;
1213
+        }
1214
+    }
1215
+  while (attempts < 6);
1185 1216
 
1186
-  return gen_path (directory, BSTR (&fname), gc);
1217
+  msg (M_FATAL, "Failed to create temporary file after %i attempts", attempts);
1218
+  return NULL;
1187 1219
 }
1188 1220
 
1189 1221
 /*
... ...
@@ -218,8 +218,8 @@ long int get_random(void);
218 218
 /* return true if filename can be opened for read */
219 219
 bool test_file (const char *filename);
220 220
 
221
-/* create a temporary filename in directory */
222
-const char *create_temp_filename (const char *directory, const char *prefix, struct gc_arena *gc);
221
+/* create a temporary file in directory, returns the filename of the created file */
222
+const char *create_temp_file (const char *directory, const char *prefix, struct gc_arena *gc);
223 223
 
224 224
 /* put a directory and filename together */
225 225
 const char *gen_path (const char *directory, const char *filename, struct gc_arena *gc);