From 4e1cc5f6dda22e9ff121d3753066775c25448bcc Mon Sep 17 00:00:00 2001 From: David Sommerseth Date: Fri, 16 Apr 2010 22:02:36 +0200 Subject: 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 Signed-off-by: Fabian Knittel Acked-by: Gert Doering --- misc.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++-------------- misc.h | 4 ++-- 2 files changed, 48 insertions(+), 16 deletions(-) diff --git a/misc.c b/misc.c index 507bcc2..0ceeb2d 100644 --- 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 328107d..d5ad774 100644 --- 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); -- cgit v1.2.3