Attachment 84404 Details for Bug 68350 – _tp_create_temp_unix_socket: avoid using tmpnam()

[patch] _tp_create_temp_unix_socket: avoid using tmpnam()

0001-_tp_create_temp_unix_socket-avoid-using-tmpnam.patch (text/plain), 4.93 KB, created by Simon McVittie on 2013-08-21 17:35:22 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2013-08-21 17:35:22 UTC

Size: 4.93 KB

patch

obsolete

>From c77aeab6164c20761f3478ba0b13c273934248ae Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Tue, 20 Aug 2013 20:42:10 +0100
>Subject: [PATCH 1/2] _tp_create_temp_unix_socket: avoid using tmpnam()
>
>On current Debian unstable, gcc/ld issues a warning about tmpnam(),
>because it's usually used in an unsafe way. "gcc -Wl,--fatal-warnings"
>(which I'm using in my development environment) upgrades that to fatal.
>
>Our usage was in fact safe (trying to listen on a socket always
>behaves like O_EXCL|O_CREAT, which can DoS'd but is not subject to
>symlink attacks), but we're swimming against the current by trying
>to use tmpnam(). Instead, create a secure private temporary directory
>with g_dir_make_tmp(), and put our socket in there.
>---
> telepathy-glib/stream-tube-channel.c | 10 ++++++-
> telepathy-glib/util-internal.h       |  1 +
> telepathy-glib/util.c                | 51 ++++++++++++++++++++++++------------
> 3 files changed, 44 insertions(+), 18 deletions(-)
>
>diff --git a/telepathy-glib/stream-tube-channel.c b/telepathy-glib/stream-tube-channel.c
>index 607abad..8a110d7 100644
>--- a/telepathy-glib/stream-tube-channel.c
>+++ b/telepathy-glib/stream-tube-channel.c
>@@ -145,6 +145,7 @@ struct _TpStreamTubeChannelPrivate
>   /* Offering side */
>   GSocketService *service;
>   GSocketAddress *address;
>+  gchar *unix_tmpdir;
>   /* GSocketConnection we have accepted but are still waiting a
>    * NewRemoteConnection to identify them. Owned ConnWaitingSig. */
>   GSList *conn_waiting_sig;
>@@ -266,6 +267,13 @@ tp_stream_tube_channel_dispose (GObject *obj)
>       self->priv->address = NULL;
>     }
> 
>+    if (self->priv->unix_tmpdir != NULL)
>+      {
>+        g_rmdir (self->priv->unix_tmpdir);
>+        g_free (self->priv->unix_tmpdir);
>+        self->priv->unix_tmpdir = NULL;
>+      }
>+
>   tp_clear_pointer (&self->priv->access_control_param, tp_g_value_slice_free);
>   tp_clear_object (&self->priv->local_conn_waiting_id);
>   tp_clear_object (&self->priv->client_socket);
>@@ -1466,7 +1474,7 @@ tp_stream_tube_channel_offer_async (TpStreamTubeChannel *self,
>       case TP_SOCKET_ADDRESS_TYPE_UNIX:
>         {
>           self->priv->address = _tp_create_temp_unix_socket (
>-              self->priv->service, &error);
>+              self->priv->service, &self->priv->unix_tmpdir, &error);
> 
>           /* check there wasn't an error on the final attempt */
>           if (self->priv->address == NULL)
>diff --git a/telepathy-glib/util-internal.h b/telepathy-glib/util-internal.h
>index 9410648..b1ecaa4 100644
>--- a/telepathy-glib/util-internal.h
>+++ b/telepathy-glib/util-internal.h
>@@ -39,6 +39,7 @@ void _tp_quark_array_merge_valist (GArray *array,
> 
> #ifdef HAVE_GIO_UNIX
> GSocketAddress * _tp_create_temp_unix_socket (GSocketService *service,
>+    gchar **tmpdir,
>     GError **error);
> #endif /* HAVE_GIO_UNIX */
> 
>diff --git a/telepathy-glib/util.c b/telepathy-glib/util.c
>index 91a42d5..c89bc66 100644
>--- a/telepathy-glib/util.c
>+++ b/telepathy-glib/util.c
>@@ -31,6 +31,7 @@
> 
> #include "config.h"
> 
>+#include <glib/gstdio.h>
> #include <gobject/gvaluecollector.h>
> 
> #ifdef HAVE_GIO_UNIX
>@@ -43,6 +44,7 @@
> #include <telepathy-glib/util-internal.h>
> #include <telepathy-glib/util.h>
> 
>+#include <errno.h>
> #include <stdio.h>
> #include <string.h>
> 
>@@ -1563,32 +1565,47 @@ _tp_quark_array_merge_valist (GArray *array,
> #ifdef HAVE_GIO_UNIX
> GSocketAddress *
> _tp_create_temp_unix_socket (GSocketService *service,
>+    gchar **tmpdir,
>     GError **error)
> {
>-  guint i;
>   GSocketAddress *address;
>+  gchar *dir = g_dir_make_tmp ("tp-glib-socket.XXXXXX", error);
>+  gchar *name;
>+
>+  if (dir == NULL)
>+    return NULL;
> 
>-  /* why doesn't GIO provide a method to create a socket we don't
>-   * care about? Iterate until we find a valid temporary name.
>-   *
>-   * Try a maximum of 10 times to get a socket */
>-  for (i = 0; i < 10; i++)
>+  if (g_chmod (dir, 0700) != 0)
>     {
>-      address = g_unix_socket_address_new (tmpnam (NULL));
>+      int e = errno;
> 
>-      g_clear_error (error);
>+      g_set_error (error, G_IO_ERROR, g_io_error_from_errno (e),
>+          "unable to set permissions of %s to 0700: %s", dir,
>+          g_strerror (e));
>+      g_free (dir);
>+      return NULL;
>+    }
> 
>-      if (g_socket_listener_add_address (
>-            G_SOCKET_LISTENER (service),
>-            address, G_SOCKET_TYPE_STREAM,
>-            G_SOCKET_PROTOCOL_DEFAULT,
>-            NULL, NULL, error))
>-        return address;
>-      else
>-        g_object_unref (address);
>+  name = g_build_filename (dir, "s", NULL);
>+  address = g_unix_socket_address_new (name);
>+  g_free (name);
>+
>+  if (!g_socket_listener_add_address (G_SOCKET_LISTENER (service),
>+        address, G_SOCKET_TYPE_STREAM,
>+        G_SOCKET_PROTOCOL_DEFAULT,
>+        NULL, NULL, error))
>+    {
>+      g_object_unref (address);
>+      g_free (dir);
>+      return NULL;
>     }
> 
>-  return NULL;
>+  if (tmpdir != NULL)
>+    *tmpdir = dir;
>+  else
>+    g_free (dir);
>+
>+  return address;
> }
> #endif /* HAVE_GIO_UNIX */
> 
>-- 
>1.8.4.rc3
>

Actions: View | Splinter Review

Attachments on bug 68350: 84404 | 84405

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.