[glib] gio: don't accept nonstandard IPv4 "numbers-and-dots" addresses
- From: Dan Winship <danw src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [glib] gio: don't accept nonstandard IPv4 "numbers-and-dots" addresses
- Date: Sat, 15 Feb 2014 15:27:15 +0000 (UTC)
commit 5575a3e9cb8ec3d0f0f373cb64e6fedc4c72c0f5
Author: Dan Winship <danw gnome org>
Date: Tue Aug 20 21:36:25 2013 -0400
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 | 1 +
gio/ginetsocketaddress.c | 74 ++++++++++++++++++++++
gio/ginetsocketaddress.h | 17 +++--
gio/gnetworkaddress.c | 28 +++------
gio/gresolver.c | 55 ++++++++++++++---
gio/gthreadedresolver.c | 2 +-
gio/tests/network-address.c | 116 +++++++++++++++++++++++++++++++++++
7 files changed, 256 insertions(+), 37 deletions(-)
---
diff --git a/docs/reference/gio/gio-sections.txt b/docs/reference/gio/gio-sections.txt
index 86e7447..ea3bd13 100644
--- a/docs/reference/gio/gio-sections.txt
+++ b/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
diff --git a/gio/ginetsocketaddress.c b/gio/ginetsocketaddress.c
index 818f877..6301e92 100644
--- a/gio/ginetsocketaddress.c
+++ b/gio/ginetsocketaddress.c
@@ -329,6 +329,80 @@ g_inet_socket_address_new (GInetAddress *address,
}
/**
+ * 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
*
diff --git a/gio/ginetsocketaddress.h b/gio/ginetsocketaddress.h
index 3a4fb58..745936e 100644
--- a/gio/ginetsocketaddress.h
+++ b/gio/ginetsocketaddress.h
@@ -54,21 +54,24 @@ struct _GInetSocketAddressClass
};
GLIB_AVAILABLE_IN_ALL
-GType g_inet_socket_address_get_type (void) G_GNUC_CONST;
+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);
+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);
+GInetAddress * g_inet_socket_address_get_address (GInetSocketAddress *address);
GLIB_AVAILABLE_IN_ALL
-guint16 g_inet_socket_address_get_port (GInetSocketAddress *address);
+guint16 g_inet_socket_address_get_port (GInetSocketAddress *address);
GLIB_AVAILABLE_IN_2_32
-guint32 g_inet_socket_address_get_flowinfo (GInetSocketAddress *address);
+guint32 g_inet_socket_address_get_flowinfo (GInetSocketAddress *address);
GLIB_AVAILABLE_IN_2_32
-guint32 g_inet_socket_address_get_scope_id (GInetSocketAddress *address);
+guint32 g_inet_socket_address_get_scope_id (GInetSocketAddress *address);
G_END_DECLS
diff --git a/gio/gnetworkaddress.c b/gio/gnetworkaddress.c
index f2f414e..094615b 100644
--- a/gio/gnetworkaddress.c
+++ b/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))
+ 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;
-
- addr->priv->sockaddrs = g_list_prepend (addr->priv->sockaddrs, sockaddr);
- return TRUE;
}
/**
diff --git a/gio/gresolver.c b/gio/gresolver.c
index 86843c6..1da8425 100644
--- a/gio/gresolver.c
+++ b/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;
}
diff --git a/gio/gthreadedresolver.c b/gio/gthreadedresolver.c
index 12a60e7..3bb4d08 100644
--- a/gio/gthreadedresolver.c
+++ b/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
diff --git a/gio/tests/network-address.c b/gio/tests/network-address.c
index cfcbd0c..a8be415 100644
--- a/gio/tests/network-address.c
+++ b/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);
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
