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