From d572bdf341bccafe55367335b0fe1c83553e6993 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Wed, 9 Oct 2013 12:08:10 +0100
Subject: [PATCH] fish: CVE-2013-4419: Fix insecure temporary directory
 handling for remote guestfish (RHBZ#1016960).

When using the guestfish --remote or guestfish --listen options,
guestfish would create a socket in a known location
(/tmp/.guestfish-$UID/socket-$PID).

The location has to be a known one in order for both ends to
communicate.  However no checking was done that the containing
directory (/tmp/.guestfish-$UID) is owned by the user.  Thus another
user could create this directory and potentially modify sockets owned
by another user's guestfish client or server.

This commit fixes the issue by creating the directory unconditionally,
and then checking that the directory has the correct owner and
permissions, thus preventing another user from creating the directory
first.

If guestfish sees a suspicious socket directory it will print an error
like this and exit with an error status:

  guestfish: '/tmp/.guestfish-1000' is not a directory or has insecure owner or permissions

Thanks: Michael Scherer for discovering this issue.

Version 2:
 - Add assigned CVE number.
 - Update documentation.

Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
---
 fish/guestfish.pod |  3 +++
 fish/rc.c          | 43 +++++++++++++++++++++++++++++++++++++++----
 src/guestfs.pod    | 17 +++++++++++++++++
 3 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/fish/guestfish.pod b/fish/guestfish.pod
index 06663ac..2ecc058 100644
--- a/fish/guestfish.pod
+++ b/fish/guestfish.pod
@@ -1006,6 +1006,9 @@ user ID of the process, and C<$PID> is the process ID of the server.
 
 Guestfish client and server versions must match exactly.
 
+Older versions of guestfish were vulnerable to CVE-2013-4419 (see
+L<guestfs(3)/CVE-2013-4419>).  This is fixed in the current version.
+
 =head2 USING REMOTE CONTROL ROBUSTLY FROM SHELL SCRIPTS
 
 From Bash, you can use the following code which creates a guestfish
diff --git a/fish/rc.c b/fish/rc.c
index aa4b849..c736042 100644
--- a/fish/rc.c
+++ b/fish/rc.c
@@ -29,6 +29,7 @@
 #include <sys/un.h>
 #include <signal.h>
 #include <sys/socket.h>
+#include <errno.h>
 
 #include <rpc/types.h>
 #include <rpc/xdr.h>
@@ -38,17 +39,49 @@
 #include "fish.h"
 #include "rc_protocol.h"
 
+/* Because this is a Unix domain socket, the total path length must be
+ * under 108 bytes.
+ */
+#define SOCKET_DIR "/tmp/.guestfish-%d" /* euid */
+#define SOCKET_PATH "/tmp/.guestfish-%d/socket-%d" /* euid, pid */
+
+static void
+create_sockdir (void)
+{
+  uid_t euid = geteuid ();
+  char dir[128];
+  int r;
+  struct stat statbuf;
+
+  /* Create the directory, and ensure it is owned by the user. */
+  snprintf (dir, sizeof dir, SOCKET_DIR, euid);
+  r = mkdir (dir, 0700);
+  if (r == -1 && errno != EEXIST) {
+  error:
+    perror (dir);
+    exit (EXIT_FAILURE);
+  }
+  if (lstat (dir, &statbuf) == -1)
+    goto error;
+  if (!S_ISDIR (statbuf.st_mode) ||
+      (statbuf.st_mode & 0777) != 0700 ||
+      statbuf.st_uid != euid) {
+    fprintf (stderr,
+             _("guestfish: '%s' is not a directory or has insecure owner or permissions\n"),
+             dir);
+    exit (EXIT_FAILURE);
+  }
+}
+
 static void
 create_sockpath (pid_t pid, char *sockpath, size_t len,
                  struct sockaddr_un *addr)
 {
-  char dir[128];
   uid_t euid = geteuid ();
 
-  snprintf (dir, sizeof dir, "/tmp/.guestfish-%d", euid);
-  ignore_value (mkdir (dir, 0700));
+  create_sockdir ();
 
-  snprintf (sockpath, len, "/tmp/.guestfish-%d/socket-%d", euid, pid);
+  snprintf (sockpath, len, SOCKET_PATH, euid, pid);
 
   addr->sun_family = AF_UNIX;
   strcpy (addr->sun_path, sockpath);
@@ -196,6 +229,8 @@ rc_listen (void)
   memset (&hello, 0, sizeof hello);
   memset (&call, 0, sizeof call);
 
+  create_sockdir ();
+
   pid = fork ();
   if (pid == -1) {
     perror ("fork");
diff --git a/src/guestfs.pod b/src/guestfs.pod
index 0c57f50..eedea94 100644
--- a/src/guestfs.pod
+++ b/src/guestfs.pod
@@ -1998,6 +1998,23 @@ double-free in the C library (denial of service).
 It is sufficient to update libguestfs to a version that is not
 vulnerable: libguestfs E<ge> 1.20.8, E<ge> 1.22.2 or E<ge> 1.23.2.
 
+=head2 CVE-2013-4419
+
+L<https://bugzilla.redhat.com/1016960>
+
+When using the L<guestfish(1)> I<--remote> or guestfish I<--listen>
+options, guestfish would create a socket in a known location
+(C</tmp/.guestfish-$UID/socket-$PID>).
+
+The location has to be a known one in order for both ends to
+communicate.  However no checking was done that the containing
+directory (C</tmp/.guestfish-$UID>) is owned by the user.  Thus
+another user could create this directory and potentially hijack
+sockets owned by another user's guestfish client or server.
+
+It is sufficient to update libguestfs to a version that is not
+vulnerable: libguestfs E<ge> 1.20.12, E<ge> 1.22.7 or E<ge> 1.24.
+
 =head1 CONNECTION MANAGEMENT
 
 =head2 guestfs_h *
-- 
1.8.3.1