daemon: Don't use files with fixed names in /tmp (thanks Steve Kemp).

Although this doesn't matter for the ordinary (appliance) case, it
matters for the libguestfs live case.  In that case it could cause the
guest to be exploited by a tmp/symlink attack.
(cherry picked from commit 6011b1f803ba7308c6a94b9bf6b7212cfccb9f42)

diff --git a/daemon/inotify.c b/daemon/inotify.c
index c8862e5..27fe4b0 100644 (file)
--- a/daemon/inotify.c
+++ b/daemon/inotify.c
@@ -1,5 +1,5 @@
 /* libguestfs - the guestfsd daemon
- * Copyright (C) 2009 Red Hat Inc.
+ * Copyright (C) 2009-2011 Red Hat Inc.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -317,10 +317,21 @@ do_inotify_files (void)
   FILE *fp = NULL;
   guestfs_int_inotify_event_list *events;
   char buf[PATH_MAX];
+  char tempfile[] = "/tmp/inotifyXXXXXX";
+  int fd;
+  char cmd[64];
 
   NEED_INOTIFY (NULL);
 
-  fp = popen ("sort -u > /tmp/inotify", "w");
+  fd = mkstemp (tempfile);
+  if (fd == -1) {
+    reply_with_perror ("mkstemp");
+    return NULL;
+  }
+
+  snprintf (cmd, sizeof cmd, "sort -u > %s", tempfile);
+
+  fp = popen (cmd, "w");
   if (fp == NULL) {
     reply_with_perror ("sort");
     return NULL;
@@ -349,9 +360,11 @@ do_inotify_files (void)
 
   pclose (fp);
 
-  fp = fopen ("/tmp/inotify", "r");
+  fp = fdopen (fd, "r");
   if (fp == NULL) {
-    reply_with_perror ("/tmp/inotify");
+    reply_with_perror ("%s", tempfile);
+    unlink (tempfile);
+    close (fd);
     return NULL;
   }
 
@@ -365,20 +378,20 @@ do_inotify_files (void)
       goto error;
   }
 
-  fclose (fp);
+  fclose (fp); /* implicitly closes fd */
   fp = NULL;
 
   if (add_string (&ret, &size, &alloc, NULL) == -1)
     goto error;
 
-  unlink ("/tmp/inotify");
+  unlink (tempfile);
   return ret;
 
  error:
   if (fp != NULL)
     fclose (fp);
 
-  unlink ("/tmp/inotify");
+  unlink (tempfile);
   return NULL;
 #else
   NOT_AVAILABLE (NULL);

diff --git a/daemon/tar.c b/daemon/tar.c
index 74d7b05..e29f440 100644 (file)
--- a/daemon/tar.c
+++ b/daemon/tar.c
@@ -1,5 +1,5 @@
 /* libguestfs - the guestfsd daemon
- * Copyright (C) 2009-2010 Red Hat Inc.
+ * Copyright (C) 2009-2011 Red Hat Inc.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -36,18 +36,14 @@ optgroup_xz_available (void)
   return prog_exists ("xz");
 }
 
-/* Redirect errors from the tar command to the error file, then
- * provide functions for reading it in.  We overwrite the file each
- * time, and since it's small and stored on the appliance we don't
- * bother to delete it.
- */
-static const char *error_file = "/tmp/error";
-
+/* Read the error file.  Returns a string that the caller must free. */
 static char *
-read_error_file (void)
+read_error_file (char *error_file)
 {
   size_t len;
-  char *str = read_file (error_file, &len);
+  char *str;
+
+  str = read_file (error_file, &len);
   if (str == NULL) {
     str = strdup ("(no error)");
     if (str == NULL) {
@@ -78,6 +74,16 @@ do_tXz_in (const char *dir, const char *filter)
   int err, r;
   FILE *fp;
   char *cmd;
+  char error_file[] = "/tmp/tarXXXXXX";
+  int fd;
+
+  fd = mkstemp (error_file);
+  if (fd == -1) {
+    reply_with_perror ("mkstemp");
+    return -1;
+  }
+
+  close (fd);
 
   /* "tar -C /sysroot%s -xf -" but we have to quote the dir. */
   if (asprintf_nowarn (&cmd, "tar -C %R -%sxf - 2> %s",
@@ -86,6 +92,7 @@ do_tXz_in (const char *dir, const char *filter)
     r = cancel_receive ();
     errno = err;
     reply_with_perror ("asprintf");
+    unlink (error_file);
     return -1;
   }
 
@@ -98,6 +105,7 @@ do_tXz_in (const char *dir, const char *filter)
     r = cancel_receive ();
     errno = err;
     reply_with_perror ("%s", cmd);
+    unlink (error_file);
     free (cmd);
     return -1;
   }
@@ -106,14 +114,15 @@ do_tXz_in (const char *dir, const char *filter)
   /* The semantics of fwrite are too undefined, so write to the
    * file descriptor directly instead.
    */
-  int fd = fileno (fp);
+  fd = fileno (fp);
 
   r = receive_file (write_cb, &fd);
   if (r == -1) {               /* write error */
     cancel_receive ();
-    char *errstr = read_error_file ();
+    char *errstr = read_error_file (error_file);
     reply_with_error ("write error on directory: %s: %s", dir, errstr);
     free (errstr);
+    unlink (error_file);
     pclose (fp);
     return -1;
   }
@@ -123,17 +132,21 @@ do_tXz_in (const char *dir, const char *filter)
      */
     reply_with_error ("file upload cancelled");
     pclose (fp);
+    unlink (error_file);
     return -1;
   }
 
   if (pclose (fp) != 0) {
-    char *errstr = read_error_file ();
+    char *errstr = read_error_file (error_file);
     reply_with_error ("tar subcommand failed on directory: %s: %s",
                       dir, errstr);
     free (errstr);
+    unlink (error_file);
     return -1;
   }
 
+  unlink (error_file);
+
   return 0;
 }