gio: don't accept nonstandard IPv4 "numbers-and-dots" addresses

In addition to the standard "192.168.1.1" format, there are numerous
legacy IPv4 address formats (such as "192.168.257",
"0xc0.0xa8.0x01.0x01", "0300.0250.0001.0001", "3232235777", and
"0xc0a80101"). However, none of these forms are ever used any more
except in phishing attempts. GLib wasn't supposed to be accepting
these addresses (neither g_hostname_is_ip_address() nor
g_inet_address_new_from_string() recognizes them), but getaddrinfo()
accepts them, and so the parts of gio that use getaddrinfo()
accidentally did accept those formats.

Fix GNetworkAddress and GResolver to reject these address formats.

https://bugzilla.gnome.org/show_bug.cgi?id=679957

docs/reference/gio/gio-sections.txt

@@ -1714,6 +1714,7 @@ g_socket_address_get_type
 <TITLE>GInetSocketAddress</TITLE>
 GInetSocketAddress
 g_inet_socket_address_new
+g_inet_socket_address_new_from_string
 g_inet_socket_address_get_address
 g_inet_socket_address_get_port
 g_inet_socket_address_get_flowinfo

gio/ginetsocketaddress.c

@@ -328,6 +328,80 @@ g_inet_socket_address_new (GInetAddress *address,
 		       NULL);
 }
 
+/**
+ * g_inet_socket_address_new_from_string:
+ * @address: the string form of an IP address
+ * @port: a port number
+ *
+ * Creates a new #GInetSocketAddress for @address and @port.
+ *
+ * If @address is an IPv6 address, it can also contain a scope ID
+ * (separated from the address by a "<literal>%</literal>").
+ *
+ * Returns: a new #GInetSocketAddress, or %NULL if @address cannot be
+ * parsed.
+ *
+ * Since: 2.40
+ */
+GSocketAddress *
+g_inet_socket_address_new_from_string (const char *address,
+                                       guint       port)
+{
+  static struct addrinfo *hints, hints_struct;
+  GSocketAddress *saddr;
+  GInetAddress *iaddr;
+  struct addrinfo *res;
+  gint status;
+
+  if (strchr (address, ':'))
+    {
+      /* IPv6 address (or it's invalid). We use getaddrinfo() because
+       * it will handle parsing a scope_id as well.
+       */
+
+      if (G_UNLIKELY (g_once_init_enter (&hints)))
+        {
+          hints_struct.ai_socktype = SOCK_STREAM;
+          hints_struct.ai_flags = AI_NUMERICHOST;
+          g_once_init_leave (&hints, &hints_struct);
+        }
+
+      status = getaddrinfo (address, NULL, hints, &res);
+      if (status != 0)
+        return NULL;
+
+      if (res->ai_family == AF_INET6 &&
+          res->ai_addrlen == sizeof (struct sockaddr_in6))
+        {
+          ((struct sockaddr_in6 *)res->ai_addr)->sin6_port = g_htons (port);
+          saddr = g_socket_address_new_from_native (res->ai_addr, res->ai_addrlen);
+        }
+      else
+        saddr = NULL;
+
+      freeaddrinfo (res);
+    }
+  else
+    {
+      /* IPv4 (or invalid). We don't want to use getaddrinfo() here,
+       * because it accepts the stupid "IPv4 numbers-and-dots
+       * notation" addresses that are never used for anything except
+       * phishing. Since we don't have to worry about scope IDs for
+       * IPv4, we can just use g_inet_address_new_from_string().
+       */
+      iaddr = g_inet_address_new_from_string (address);
+      if (!iaddr)
+        return NULL;
+
+      g_warn_if_fail (g_inet_address_get_family (iaddr) == G_SOCKET_FAMILY_IPV4);
+
+      saddr = g_inet_socket_address_new (iaddr, port);
+      g_object_unref (iaddr);
+    }
+
+  return saddr;
+}
+
 /**
  * g_inet_socket_address_get_address:
  * @address: a #GInetSocketAddress

gio/ginetsocketaddress.h

@@ -59,6 +59,9 @@ GType           g_inet_socket_address_get_type     (void) G_GNUC_CONST;
 GLIB_AVAILABLE_IN_ALL
 GSocketAddress *g_inet_socket_address_new             (GInetAddress       *address,
                                                        guint16             port);
+GLIB_AVAILABLE_IN_2_40
+GSocketAddress *g_inet_socket_address_new_from_string (const char         *address,
+                                                       guint               port);
 
 GLIB_AVAILABLE_IN_ALL
 GInetAddress *  g_inet_socket_address_get_address     (GInetSocketAddress *address);

gio/gnetworkaddress.c

@@ -247,29 +247,17 @@ g_network_address_set_addresses (GNetworkAddress *addr,
 static gboolean
 g_network_address_parse_sockaddr (GNetworkAddress *addr)
 {
-  struct addrinfo hints, *res = NULL;
   GSocketAddress *sockaddr;
-  gchar port[32];
-
-  memset (&hints, 0, sizeof (hints));
-  hints.ai_socktype = SOCK_STREAM;
-  hints.ai_flags = AI_NUMERICHOST
-#ifdef AI_NUMERICSERV
-    | AI_NUMERICSERV
-#endif
-    ;
-  g_snprintf (port, sizeof (port), "%u", addr->priv->port);
-
-  if (getaddrinfo (addr->priv->hostname, port, &hints, &res) != 0)
-    return FALSE;
-
-  sockaddr = g_socket_address_new_from_native (res->ai_addr, res->ai_addrlen);
-  freeaddrinfo (res);
-  if (!sockaddr || !G_IS_INET_SOCKET_ADDRESS (sockaddr))
-    return FALSE;
 
+  sockaddr = g_inet_socket_address_new_from_string (addr->priv->hostname,
+                                                    addr->priv->port);
+  if (sockaddr)
+    {
       addr->priv->sockaddrs = g_list_prepend (addr->priv->sockaddrs, sockaddr);
       return TRUE;
+    }
+  else
+    return FALSE;
 }
 
 /**

gio/gresolver.c

@@ -283,6 +283,43 @@ remove_duplicates (GList *addrs)
     }
 }
 
+/* Note that this does not follow the "FALSE means @error is set"
+ * convention. The return value tells the caller whether it should
+ * return @addrs and @error to the caller right away, or if it should
+ * continue and trying to resolve the name as a hostname.
+ */
+static gboolean
+handle_ip_address (const char  *hostname,
+                   GList      **addrs,
+                   GError     **error)
+{
+  GInetAddress *addr;
+  struct in_addr ip4addr;
+
+  addr = g_inet_address_new_from_string (hostname);
+  if (addr)
+    {
+      *addrs = g_list_append (NULL, addr);
+      return TRUE;
+    }
+
+  *addrs = NULL;
+
+  /* Reject non-standard IPv4 numbers-and-dots addresses.
+   * g_inet_address_new_from_string() will have accepted any "real" IP
+   * address, so if inet_aton() succeeds, then it's an address we want
+   * to reject.
+   */
+  if (inet_aton (hostname, &ip4addr))
+    {
+      g_set_error (error, G_RESOLVER_ERROR, G_RESOLVER_ERROR_NOT_FOUND,
+                   _("Error resolving '%s': %s"),
+                   hostname, gai_strerror (EAI_NODATA));
+      return TRUE;
+    }
+
+  return FALSE;
+}
 
 /**
  * g_resolver_lookup_by_name:
@@ -328,7 +365,6 @@ g_resolver_lookup_by_name (GResolver     *resolver,
                            GCancellable  *cancellable,
                            GError       **error)
 {
-  GInetAddress *addr;
   GList *addrs;
   gchar *ascii_hostname = NULL;
 
@@ -336,9 +372,8 @@ g_resolver_lookup_by_name (GResolver     *resolver,
   g_return_val_if_fail (hostname != NULL, NULL);
 
   /* Check if @hostname is just an IP address */
-  addr = g_inet_address_new_from_string (hostname);
-  if (addr)
-    return g_list_append (NULL, addr);
+  if (handle_ip_address (hostname, &addrs, error))
+    return addrs;
 
   if (g_hostname_is_non_ascii (hostname))
     hostname = ascii_hostname = g_hostname_to_ascii (hostname);
@@ -375,22 +410,24 @@ g_resolver_lookup_by_name_async (GResolver           *resolver,
                                  GAsyncReadyCallback  callback,
                                  gpointer             user_data)
 {
-  GInetAddress *addr;
   gchar *ascii_hostname = NULL;
+  GList *addrs;
+  GError *error = NULL;
 
   g_return_if_fail (G_IS_RESOLVER (resolver));
   g_return_if_fail (hostname != NULL);
 
   /* Check if @hostname is just an IP address */
-  addr = g_inet_address_new_from_string (hostname);
-  if (addr)
+  if (handle_ip_address (hostname, &addrs, &error))
     {
       GTask *task;
 
       task = g_task_new (resolver, cancellable, callback, user_data);
       g_task_set_source_tag (task, g_resolver_lookup_by_name_async);
-      g_task_return_pointer (task, g_list_append (NULL, addr),
-                             (GDestroyNotify) g_resolver_free_addresses);
+      if (addrs)
+        g_task_return_pointer (task, addrs, (GDestroyNotify) g_resolver_free_addresses);
+      else
+        g_task_return_error (task, error);
       g_object_unref (task);
       return;
     }

gio/gthreadedresolver.c

@@ -925,7 +925,7 @@ g_threaded_resolver_class_init (GThreadedResolverClass *threaded_class)
   resolver_class->lookup_records_async     = lookup_records_async;
   resolver_class->lookup_records_finish    = lookup_records_finish;
 
-  /* Initialize _g_resolver_addrinfo_hints */
+  /* Initialize addrinfo_hints */
 #ifdef AI_ADDRCONFIG
   addrinfo_hints.ai_flags |= AI_ADDRCONFIG;
 #endif

gio/tests/network-address.c

@@ -113,6 +113,108 @@ test_parse_host (gconstpointer d)
     g_error_free (error);
 }
 
+typedef struct {
+  const gchar *input;
+  gboolean valid_parse, valid_resolve, valid_ip;
+} ResolveTest;
+
+static ResolveTest address_tests[] = {
+  { "192.168.1.2",         TRUE,  TRUE,  TRUE },
+  { "fe80::42",            TRUE,  TRUE,  TRUE },
+
+  /* GResolver accepts this by ignoring the scope ID. This was not
+   * intentional, but it's best to not "fix" it at this point.
+   */
+  { "fe80::42%1",          TRUE,  TRUE,  FALSE },
+
+  /* g_network_address_parse() accepts these, but they are not
+   * (just) IP addresses.
+   */
+  { "192.168.1.2:80",      TRUE,  FALSE, FALSE },
+  { "[fe80::42]",          TRUE,  FALSE, FALSE },
+  { "[fe80::42]:80",       TRUE,  FALSE, FALSE },
+
+  /* These should not be considered IP addresses by anyone. */
+  { "192.168.258",         FALSE, FALSE, FALSE },
+  { "192.11010306",        FALSE, FALSE, FALSE },
+  { "3232235778",          FALSE, FALSE, FALSE },
+  { "0300.0250.0001.0001", FALSE, FALSE, FALSE },
+  { "0xC0.0xA8.0x01.0x02", FALSE, FALSE, FALSE },
+  { "0xc0.0xa8.0x01.0x02", FALSE, FALSE, FALSE },
+  { "0xc0a80102",          FALSE, FALSE, FALSE }
+};
+
+static void
+test_resolve_address (gconstpointer d)
+{
+  const ResolveTest *test = d;
+  GSocketConnectable *connectable;
+  GSocketAddressEnumerator *addr_enum;
+  GSocketAddress *addr;
+  GError *error = NULL;
+
+  g_assert_cmpint (test->valid_ip, ==, g_hostname_is_ip_address (test->input));
+
+  connectable = g_network_address_parse (test->input, 1234, &error);
+  g_assert_no_error (error);
+
+  addr_enum = g_socket_connectable_enumerate (connectable);
+  addr = g_socket_address_enumerator_next (addr_enum, NULL, &error);
+  g_object_unref (addr_enum);
+  g_object_unref (connectable);
+
+  if (addr)
+    {
+      g_assert_true (test->valid_parse);
+      g_assert_true (G_IS_INET_SOCKET_ADDRESS (addr));
+      g_object_unref (addr);
+    }
+  else
+    {
+      g_assert_false (test->valid_parse);
+      g_assert_error (error, G_RESOLVER_ERROR, G_RESOLVER_ERROR_NOT_FOUND);
+      g_error_free (error);
+      return;
+    }
+}
+
+/* Technically this should be in a GResolver test program, but we don't
+ * have one of those since it's mostly impossible to test programmatically.
+ * So it goes here so it can share the tests.
+ */
+static void
+test_resolve_address_gresolver (gconstpointer d)
+{
+  const ResolveTest *test = d;
+  GResolver *resolver;
+  GList *addrs;
+  GInetAddress *iaddr;
+  GError *error = NULL;
+
+  resolver = g_resolver_get_default ();
+  addrs = g_resolver_lookup_by_name (resolver, test->input, NULL, &error);
+  g_object_unref (resolver);
+
+  if (addrs)
+    {
+      g_assert_true (test->valid_resolve);
+      g_assert_cmpint (g_list_length (addrs), ==, 1);
+
+      iaddr = addrs->data;
+      g_assert_true (G_IS_INET_ADDRESS (iaddr));
+
+      g_object_unref (iaddr);
+      g_list_free (addrs);
+    }
+  else
+    {
+      g_assert_false (test->valid_resolve);
+      g_assert_error (error, G_RESOLVER_ERROR, G_RESOLVER_ERROR_NOT_FOUND);
+      g_error_free (error);
+      return;
+    }
+}
+
 #define SCOPE_ID_TEST_ADDR "fe80::42"
 #define SCOPE_ID_TEST_PORT 99
 
@@ -252,6 +354,20 @@ main (int argc, char *argv[])
       g_free (path);
     }
 
+  for (i = 0; i < G_N_ELEMENTS (address_tests); i++)
+    {
+      path = g_strdup_printf ("/network-address/resolve-address/%d", i);
+      g_test_add_data_func (path, &address_tests[i], test_resolve_address);
+      g_free (path);
+    }
+
+  for (i = 0; i < G_N_ELEMENTS (address_tests); i++)
+    {
+      path = g_strdup_printf ("/gresolver/resolve-address/%d", i);
+      g_test_add_data_func (path, &address_tests[i], test_resolve_address_gresolver);
+      g_free (path);
+    }
+
   g_test_add_func ("/network-address/scope-id", test_host_scope_id);
   g_test_add_func ("/network-address/uri-scope-id", test_uri_scope_id);