[Spice-commits] 2 commits - configure.ac gtk/spice-client-glib-usb-acl-helper.c gtk/spice-session.c

[Christophe Fergau]

 configure.ac                           |    2 ++
 gtk/spice-client-glib-usb-acl-helper.c |   24 +++++++++++++++++++++++-
 gtk/spice-session.c                    |   17 ++++++++++-------
 3 files changed, 35 insertions(+), 8 deletions(-)

New commits:
commit 504e57044af905a21d89a6effe3a3774eaad3634
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Sep 20 10:41:39 2012 +0200

    Unescape SpiceSession::uri component by component
    
    Unescaping the whole URI and then parsing it is dangerous as
    the unescaping may (for example) add some extra '/' in the URI
    which are not part of a path. It's better to do the unescaping later
    once the URI has been split in separate components.
    This commit unescapes the path, host and query values. Handling escaped
    query values is important for usernames/passwords which might contain
    chars which are invalid in URIs.
    If the host is enclosed in [], it's intentionally not escaped as this
    contains an ipv6 URI, and may contain a %zone_id (see RFC4007). This is
    consistent with libvirt/libxml2 behaviour, not with what gvfs does.

diff --git a/gtk/spice-session.c b/gtk/spice-session.c
index 5fbc1d2..68d1594 100644
--- a/gtk/spice-session.c
+++ b/gtk/spice-session.c
@@ -247,18 +247,15 @@ static int spice_uri_create(SpiceSession *session, char *dest, int len)
 static int spice_uri_parse(SpiceSession *session, const char *original_uri)
 {
     SpiceSessionPrivate *s = SPICE_SESSION_GET_PRIVATE(session);
-    gchar key[32], value[128];
     gchar *host = NULL, *port = NULL, *tls_port = NULL, *uri = NULL, *password = NULL;
-    gchar **target_key;
     gchar *path = NULL;
+    gchar *unescaped_path = NULL;
     gchar *authority = NULL;
     gchar *query = NULL;
 
     g_return_val_if_fail(original_uri != NULL, -1);
 
-    uri = g_uri_unescape_string(original_uri, NULL);
-    g_return_val_if_fail(uri != NULL, -1);
-
+    uri = g_strdup(original_uri);
 
     /* Break up the URI into its various parts, scheme, authority,
      * path (ignored) and query
@@ -308,7 +305,7 @@ static int spice_uri_parse(SpiceSession *session, const char *original_uri)
             tmp++;
             port = g_strdup(tmp);
         }
-        host = g_strdup(authority);
+        host = g_uri_unescape_string(authority, NULL);
     }
 
     if (path && !(g_str_equal(path, "") ||
@@ -316,8 +313,12 @@ static int spice_uri_parse(SpiceSession *session, const char *original_uri)
         g_warning("Unexpected path data '%s' for URI '%s'", path, uri);
         /* don't fail, just ignore */
     }
+    unescaped_path = g_uri_unescape_string(path, NULL);
 
     while (query && query[0] != '\0') {
+        gchar key[32], value[128];
+        gchar **target_key;
+
         int len;
         if (sscanf(query, "%31[-a-zA-Z0-9]=%127[^;&]%n", key, value, &len) != 2) {
             g_warning("Failed to parse URI query '%s'", query);
@@ -344,7 +345,7 @@ static int spice_uri_parse(SpiceSession *session, const char *original_uri)
                 g_warning("Double set of '%s' in URI '%s'", key, uri);
                 goto fail;
             }
-            *target_key = g_strdup(value);
+            *target_key = g_uri_unescape_string(value, NULL);
         }
     }
 
@@ -355,6 +356,7 @@ static int spice_uri_parse(SpiceSession *session, const char *original_uri)
 
     /* parsed ok -> apply */
     g_free(uri);
+    g_free(unescaped_path);
     g_free(s->host);
     g_free(s->port);
     g_free(s->tls_port);
@@ -367,6 +369,7 @@ static int spice_uri_parse(SpiceSession *session, const char *original_uri)
 
 fail:
     g_free(uri);
+    g_free(unescaped_path);
     g_free(host);
     g_free(port);
     g_free(tls_port);
commit efbf867bb88845d5edf839550b54494b1bb752b9
Author: Colin Walters <walters at verbum.org>
Date:   Fri Sep 14 11:21:28 2012 +0200

    usb-acl-helper: Clear environment
    
    Otherwise we can be subject to attack via environment variables such
    as DBUS_SYSTEM_BUS_ADDRESS.
    This addresses CVE-2012-4425 http://seclists.org/oss-sec/2012/q3/470

diff --git a/configure.ac b/configure.ac
index 4a220d1..c7367cc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -244,6 +244,8 @@ else
         EXTERNAL_PNP_IDS="$with_pnp_ids_path"
 fi
 
+AC_CHECK_FUNCS(clearenv)
+
 PKG_CHECK_MODULES(GLIB2, glib-2.0 >= 2.22)
 AC_SUBST(GLIB2_CFLAGS)
 AC_SUBST(GLIB2_LIBS)
diff --git a/gtk/spice-client-glib-usb-acl-helper.c b/gtk/spice-client-glib-usb-acl-helper.c
index 724d62a..93b9b3a 100644
--- a/gtk/spice-client-glib-usb-acl-helper.c
+++ b/gtk/spice-client-glib-usb-acl-helper.c
@@ -158,7 +158,8 @@ static void cleanup(void)
     if (state == STATE_WAITING_FOR_STDIN_EOF)
         set_facl(path, getuid(), 0);
 
-    g_main_loop_quit(loop);
+    if (loop)
+        g_main_loop_quit(loop);
 }
 
 /* Not available in polkit < 0.101 */
@@ -311,11 +312,32 @@ polkit_authority_get_sync (GCancellable *cancellable, GError **error)
 }
 #endif
 
+#ifndef HAVE_CLEARENV
+extern char **environ;
+
+static int
+clearenv (void)
+{
+        if (environ != NULL)
+                environ[0] = NULL;
+        return 0;
+}
+#endif
+
 int main(void)
 {
     pid_t parent_pid;
     GInputStream *stdin_unix_stream;
 
+  /* Nuke the environment to get a well-known and sanitized
+   * environment to avoid attacks via e.g. the DBUS_SYSTEM_BUS_ADDRESS
+   * environment variable and similar.
+   */
+    if (clearenv () != 0) {
+        FATAL_ERROR("Error clearing environment: %s\n", g_strerror (errno));
+        return 1;
+    }
+
     g_type_init();
 
     loop = g_main_loop_new(NULL, FALSE);