This fixes CVE-2007-3381 - a denial of service attack where the user can

2007-07-30  Brian Cameron  <brian.cameron@sun.com>

        This fixes CVE-2007-3381 - a denial of service attack where
        the user can crash the GDM daemon with a carefully crafted GDM
        sockets command and cause GDM to stop managing future displays.

        * daemon/gdm.c, daemon/gdmconfig.c, gui/gdmconfig.c,
          gui/gdmflexiserver.c, gui/gdmconfig.c: Fix g_strsplit calls
          so that NULL return codes are better handled.

svn path=/branches/gnome-2-16/; revision=5100

6 files changed, 47 insertions, 23 deletions

diff --git a/ChangeLog b/ChangeLog
index 674f724b..ce2c4adb 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2007-07-30  Brian Cameron  <brian.cameron@sun.com>
+
+	This fixes CVE-2007-3381 - a denial of service attack where
+	the user can crash the GDM daemon with a carefully crafted GDM
+	sockets command and cause GDM to stop managing future displays.
+
+	* daemon/gdm.c, daemon/gdmconfig.c, gui/gdmconfig.c,
+	  gui/gdmflexiserver.c, gui/gdmconfig.c: Fix g_strsplit calls
+	  so that NULL return codes are better handled.
+
 2007-04-09  Brian Cameron  <brian.cameron@sun.com>
 
         * Release 2.16.6:
diff --git a/daemon/gdm.c b/daemon/gdm.c
index de95cc94..6e08e9d4 100644
--- a/daemon/gdm.c
+++ b/daemon/gdm.c
@@ -3087,9 +3087,13 @@ gdm_handle_user_message (GdmConnection *conn, const gchar *msg, gpointer data)
  
 	} else if (strncmp (msg, GDM_SUP_GET_SERVER_DETAILS " ",
 		     strlen (GDM_SUP_GET_SERVER_DETAILS " ")) == 0) {
-		const gchar *server = &msg[strlen (GDM_SUP_GET_SERVER_DETAILS " ")];
-		gchar   **splitstr = g_strsplit (server, " ", 2);
-		GdmXserver  *svr   = gdm_find_xserver ((gchar *)splitstr[0]);
+		const gchar  *server   = &msg[strlen (GDM_SUP_GET_SERVER_DETAILS " ")];
+		gchar       **splitstr = g_strsplit (server, " ", 2);
+		GdmXserver   *svr      = NULL;
+
+		if (splitstr != NULL && splitstr[0] != NULL) {
+			svr = gdm_find_xserver ((gchar *)splitstr[0]);
+		}
 
 		if (svr != NULL) {
 			if (g_strcasecmp (splitstr[1], "ID") == 0)
@@ -3126,12 +3130,11 @@ gdm_handle_user_message (GdmConnection *conn, const gchar *msg, gpointer data)
 			   gdm_connection_printf (conn, "OK false\n");
 			else
 			   gdm_connection_printf (conn, "ERROR 2 Key not valid\n");
-
-			g_strfreev (splitstr);
 		} else {
                		gdm_connection_printf (conn, "ERROR 1 Server not found\n");
 		}
  
+		g_strfreev (splitstr);
 	} else if (strcmp (msg, GDM_SUP_GREETERPIDS) == 0) {
 		GString *msg;
 		GSList *li;
@@ -3161,10 +3164,15 @@ gdm_handle_user_message (GdmConnection *conn, const gchar *msg, gpointer data)
 	} else if (strncmp (msg, GDM_SUP_GET_CONFIG " ",
 		     strlen (GDM_SUP_GET_CONFIG " ")) == 0) {
 		const gchar *parms = &msg[strlen (GDM_SUP_GET_CONFIG " ")];
-		gchar **splitstr = g_strsplit (parms, " ", 2);
-		gchar *retval = NULL;
+		gchar **splitstr   = g_strsplit (parms, " ", 2);
+		gchar *retval      = NULL;
 		static gboolean done_prefetch = FALSE;
 
+		if (splitstr == NULL || splitstr[0] == NULL) {
+               		gdm_connection_printf (conn, "ERROR 50 Unsupported key <null>\n");
+			return;
+		}
+		
 		/*
 		 * It is not meaningful to manage this in a per-display 
 		 * fashion since the prefetch program is only run once the
diff --git a/daemon/gdmconfig.c b/daemon/gdmconfig.c
index df210246..1ff4024a 100644
--- a/daemon/gdmconfig.c
+++ b/daemon/gdmconfig.c
@@ -816,9 +816,10 @@ gdm_config_key_to_string_per_display (const gchar *display, gchar *key, gchar **
 
    file = gdm_get_per_display_custom_config_file (display);
 
-   if (strcmp (ve_sure_string (splitstr[0]), "greeter") == 0 ||
-       strcmp (ve_sure_string (splitstr[0]), "gui") == 0 ||
-       is_key (key, GDM_KEY_PAM_STACK)) {
+   if (splitstr != NULL &&
+       (strcmp (ve_sure_string (splitstr[0]), "greeter") == 0 ||
+        strcmp (ve_sure_string (splitstr[0]), "gui") == 0 ||
+        is_key (key, GDM_KEY_PAM_STACK))) {
       gdm_config_key_to_string (file, key, retval);
    }
 
@@ -844,7 +845,7 @@ gdm_config_key_to_string (gchar *file, gchar *key, gchar **retval)
    *retval = NULL;
 
    /* Should not fail, all keys should have a category. */
-   if (splitstr[0] == NULL)
+   if (splitstr == NULL || splitstr[0] == NULL)
       return;
 
    /* If file doesn't exist, then just return */
@@ -1705,7 +1706,7 @@ gdm_update_config (gchar* key)
    if (custom_cfg != NULL) {
        gchar **splitstr = g_strsplit (key, "/", 2);
 
-       if (splitstr[0] != NULL) {
+       if (splitstr != NULL && splitstr[0] != NULL) {
           GList *list = ve_config_get_keys (custom_cfg, splitstr[0]);
 
           while (list != NULL) {
@@ -1893,7 +1894,7 @@ gdm_load_config_option (gpointer key_in, gpointer value_in, gpointer data)
       /* First check the custom file */
       if (cfgfiles->custom_cfg != NULL) {
           gchar **splitstr = g_strsplit (key_in, "/", 2);
-          if (splitstr[0] != NULL) {
+          if (splitstr != NULL && splitstr[0] != NULL) {
              GList *list = ve_config_get_keys (cfgfiles->custom_cfg, splitstr[0]);
 
              while (list != NULL) {
diff --git a/gui/gdmconfig.c b/gui/gdmconfig.c
index 24e222f6..9586e446 100644
--- a/gui/gdmconfig.c
+++ b/gui/gdmconfig.c
@@ -214,11 +214,11 @@ gdm_config_get_xservers (gboolean flexible)
 	}
 
 	/* skip the "OK " */
-        splitstr = g_strsplit (result + 3, ";", 0);
-	sec = splitstr;
+	splitstr = g_strsplit (result + 3, ";", 0);
+	sec      = splitstr;
 	g_free (result);
 
-        while (*sec != NULL) {
+	while (sec != NULL && *sec != NULL) {
 		GdmXserver *svr = g_new0 (GdmXserver, 1);
 
 		temp = gdm_config_get_xserver_details (*sec, "ID");
diff --git a/gui/gdmflexiserver.c b/gui/gdmflexiserver.c
index 176551a1..2ead4f1b 100644
--- a/gui/gdmflexiserver.c
+++ b/gui/gdmflexiserver.c
@@ -136,9 +136,10 @@ get_vt_num (char **vec, char *vtpart, int depth)
 	for (i = 0; vec[i] != NULL; i++) {
 		char **rvec;
 		rvec = g_strsplit (vec[i], ",", -1);
-		if (rvec == NULL ||
-		    ve_vector_len (rvec) != 3)
+		if (ve_vector_len (rvec) != 3) {
+			g_strfreev (rvec);
 			continue;
+		}
 
 		if (strcmp (rvec[0], vtpart) == 0) {
 			/* could be nested? */
@@ -177,9 +178,10 @@ create_model (char **vec)
 		char **rvec;
 		int vt;
 		rvec = g_strsplit (vec[i], ",", -1);
-		if (rvec == NULL ||
-		    ve_vector_len (rvec) != 3)
+		if (ve_vector_len (rvec) != 3) {
+			g_strfreev (rvec);
 			continue;
+		}
 
 		vt = get_vt_num (vec, rvec[2], 5);
 
@@ -516,9 +518,10 @@ check_for_users (void)
 		char **rvec;
 		int vt;
 		rvec = g_strsplit (vec[i], ",", -1);
-		if (rvec == NULL ||
-		    ve_vector_len (rvec) != 3)
+		if (ve_vector_len (rvec) != 3) {
+			g_strfreev (rvec);
 			continue;
+		}
 
 		vt = get_vt_num (vec, rvec[2], 5);
 
diff --git a/gui/greeter/greeter_item_ulist.c b/gui/greeter/greeter_item_ulist.c
index 5f5fddc9..b1c382d0 100644
--- a/gui/greeter/greeter_item_ulist.c
+++ b/gui/greeter/greeter_item_ulist.c
@@ -146,8 +146,10 @@ check_for_displays (void)
 		char **rvec;
 
 		rvec = g_strsplit (vec[i], ",", -1);
-		if (rvec == NULL || ve_vector_len (rvec) != 3)
+		if (ve_vector_len (rvec) != 3) {
+			g_strfreev (rvec);
 			continue;
+		}
 
 		g_hash_table_insert (displays_hash,
 				     g_strdup (rvec[1]),