The function gdm_address_new_from_sockaddr_storage gets called in a fewPOST_SWITCH_TO_GOBJECT_BRANCH

2007-10-15  William Jon McCann  <mccann@jhu.edu>

	* common/gdm-address.c: (gdm_address_new_from_sockaddr),
	(gdm_address_peek_local_list):
	* common/gdm-address.h:
	* daemon/gdm-xdmcp-display-factory.c: (do_bind),
	(create_address_from_request), (decode_packet):
	* gui/simple-chooser/gdm-host-chooser-widget.c: (decode_packet),
	(find_broadcast_addresses), (add_hosts):
	The function gdm_address_new_from_sockaddr_storage gets called in a
	few places with socket addresses that aren't necessary
	sockaddr_storage bytes big (all the places that call getaddrinfo).
	This results in the memdup call in that function potentially copying
	out of bounds bytes.
	Patch from: Ray Strode <halfline@gmail.com>


svn path=/branches/mccann-gobject/; revision=5360

5 files changed, 35 insertions, 13 deletions

diff --git a/ChangeLog b/ChangeLog
index 24a9b505..559f3e6f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,21 @@
 2007-10-15  William Jon McCann  <mccann@jhu.edu>
 
+	* common/gdm-address.c: (gdm_address_new_from_sockaddr),
+	(gdm_address_peek_local_list):
+	* common/gdm-address.h:
+	* daemon/gdm-xdmcp-display-factory.c: (do_bind),
+	(create_address_from_request), (decode_packet):
+	* gui/simple-chooser/gdm-host-chooser-widget.c: (decode_packet),
+	(find_broadcast_addresses), (add_hosts):
+	The function gdm_address_new_from_sockaddr_storage gets called in a
+	few places with socket addresses that aren't necessary
+	sockaddr_storage bytes big (all the places that call getaddrinfo).
+	This results in the memdup call in that function potentially copying
+	out of bounds bytes.
+	Patch from: Ray Strode <halfline@gmail.com>
+
+2007-10-15  William Jon McCann  <mccann@jhu.edu>
+
 	* gui/simple-greeter/gdm-greeter-background.c (update_background):
 	Don't crash if background pattern can't be loaded.
 	Patch from: Ray Strode <halfline@gmail.com>
diff --git a/common/gdm-address.c b/common/gdm-address.c
index 2330fc59..ec488b23 100644
--- a/common/gdm-address.c
+++ b/common/gdm-address.c
@@ -84,22 +84,27 @@ gdm_address_get_family_type (GdmAddress *address)
 
 /**
  * gdm_address_new_from_sockaddr:
- * @sa: A pointer to a sockaddr_storage.
+ * @sa: A pointer to a sockaddr.
+ * @size: size of sockaddr in bytes.
  *
- * Creates a new #GdmAddress from @ss.
+ * Creates a new #GdmAddress from @sa.
  *
  * Return value: The new #GdmAddress
  * or %NULL if @sa was invalid or the address family isn't supported.
  **/
 GdmAddress *
-gdm_address_new_from_sockaddr_storage (struct sockaddr_storage *ss)
+gdm_address_new_from_sockaddr (struct sockaddr *sa,
+                               size_t           size)
 {
         GdmAddress *addr;
 
-        g_return_val_if_fail (ss != NULL, NULL);
+        g_return_val_if_fail (sa != NULL, NULL);
+        g_return_val_if_fail (size >= sizeof (struct sockaddr), NULL);
+        g_return_val_if_fail (size <= sizeof (struct sockaddr_storage), NULL);
 
         addr = g_new0 (GdmAddress, 1);
-        addr->ss = g_memdup (ss, sizeof (struct sockaddr_storage));
+        addr->ss = g_new0 (struct sockaddr_storage, 1);
+        memcpy (addr->ss, sa, size);
 
         return addr;
 }
@@ -315,7 +320,7 @@ gdm_address_peek_local_list (void)
         for (res = result; res != NULL; res = res->ai_next) {
                 GdmAddress *address;
 
-                address = gdm_address_new_from_sockaddr_storage ((struct sockaddr_storage *)res->ai_addr);
+                address = gdm_address_new_from_sockaddr (res->ai_addr, res->ai_addrlen);
                 the_list = g_list_append (the_list, address);
         }
 
diff --git a/common/gdm-address.h b/common/gdm-address.h
index 2bebedbb..5a01f8fa 100644
--- a/common/gdm-address.h
+++ b/common/gdm-address.h
@@ -40,7 +40,8 @@ typedef struct _GdmAddress GdmAddress;
 
 GType                    gdm_address_get_type                  (void);
 
-GdmAddress *             gdm_address_new_from_sockaddr_storage (struct sockaddr_storage *ss);
+GdmAddress *             gdm_address_new_from_sockaddr         (struct sockaddr *sa,
+                                                                size_t           size);
 
 int                      gdm_address_get_family_type           (GdmAddress              *address);
 struct sockaddr_storage *gdm_address_get_sockaddr_storage      (GdmAddress              *address);
diff --git a/daemon/gdm-xdmcp-display-factory.c b/daemon/gdm-xdmcp-display-factory.c
index 3bea1829..71ef728c 100644
--- a/daemon/gdm-xdmcp-display-factory.c
+++ b/daemon/gdm-xdmcp-display-factory.c
@@ -446,7 +446,7 @@ do_bind (guint                     port,
                         char       *serv;
                         GdmAddress *addr;
 
-                        addr = gdm_address_new_from_sockaddr_storage ((struct sockaddr_storage *)ai->ai_addr);
+                        addr = gdm_address_new_from_sockaddr (ai->ai_addr, ai->ai_addrlen);
 
                         host = NULL;
                         serv = NULL;
@@ -1356,7 +1356,7 @@ create_address_from_request (ARRAY8      *req_addr,
         if (ai != NULL) {
                 found = TRUE;
                 if (address != NULL) {
-                        *address = gdm_address_new_from_sockaddr_storage ((struct sockaddr_storage *)ai->ai_addr);
+                        *address = gdm_address_new_from_sockaddr (ai->ai_addr, ai->ai_addrlen);
                 }
         }
 
@@ -2629,7 +2629,7 @@ decode_packet (GIOChannel             *source,
                 return TRUE;
         }
 
-        address = gdm_address_new_from_sockaddr_storage (&clnt_ss);
+        address = gdm_address_new_from_sockaddr ((struct sockaddr *) &clnt_ss, ss_len);
         if (address == NULL) {
                 g_warning (_("XMDCP: Unable to parse address"));
                 return TRUE;
diff --git a/gui/simple-chooser/gdm-host-chooser-widget.c b/gui/simple-chooser/gdm-host-chooser-widget.c
index f7dae20b..90a83e81 100644
--- a/gui/simple-chooser/gdm-host-chooser-widget.c
+++ b/gui/simple-chooser/gdm-host-chooser-widget.c
@@ -237,7 +237,7 @@ decode_packet (GIOChannel           *source,
                 return TRUE;
         }
 
-        address = gdm_address_new_from_sockaddr_storage (&clnt_ss);
+        address = gdm_address_new_from_sockaddr ((struct sockaddr *) &clnt_ss, ss_len);
         if (address == NULL) {
                 g_warning (_("XMDCP: Unable to parse address"));
                 return TRUE;
@@ -462,7 +462,7 @@ find_broadcast_addresses (GdmHostChooserWidget *widget)
 
                         g_memmove (&sin, &ifreq.ifr_broadaddr, sizeof (struct sockaddr_in));
                         sin.sin_port = htons (XDM_UDP_PORT);
-                        address = gdm_address_new_from_sockaddr_storage ((struct sockaddr_storage *)&sin);
+                        address = gdm_address_new_from_sockaddr ((struct sockaddr *) &sin, sizeof (sin));
                         if (address != NULL) {
                                 g_debug ("Adding if %s", name);
                                 gdm_address_debug (address);
@@ -518,7 +518,7 @@ add_hosts (GdmHostChooserWidget *widget)
                 for (ai = result; ai != NULL; ai = ai->ai_next) {
                         GdmAddress *address;
 
-                        address = gdm_address_new_from_sockaddr_storage ((struct sockaddr_storage *)ai->ai_addr);
+                        address = gdm_address_new_from_sockaddr (ai->ai_addr, ai->ai_addrlen);
                         if (address != NULL) {
                                 widget->priv->query_addresses = g_slist_append (widget->priv->query_addresses, address);
                         }