[libsoup/gnome-3-28] soup-tld: check for trailing dots in domains

[Claudio Saavedra <csaavedra src gnome org>]

commit f7fb41f971cbdca17f49fd31c2ecbb853a029d68
Author: Claudio Saavedra <csaavedra igalia com>
Date:   Wed Aug 8 17:51:28 2018 +0300

    soup-tld: check for trailing dots in domains
    
    SoupCookieJar relies on SoupTLD to check whether cookies are for
    toplevel domains. Up to this point, SoupTLD does not consider possible
    trailing dots in domains, so a toplevel domain with a trailing dot
    will be discarded (because it doesn't string-match any of the TLDs in
    soup's copy of the public-suffix list). SoupCookieJar will then
    erroneously believe that a TLD with a trailing dot is not a TLD and
    will accept the cookie, which will later be passed to other domains
    with the same TLD.
    
    Fix this by discarding the trailing dot before looking up domains in
    the public-suffix list. This way SoupTLD will work correctly with
    domains with a trailing dot and SoupCookieJar will reject them if they
    are TLDs.
    
    https://gitlab.gnome.org/GNOME/libsoup/issues/5

 libsoup/soup-tld.c   | 12 +++++++++-
 tests/cookies-test.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++---
 tests/tld-test.c     |  3 +++
 3 files changed, 77 insertions(+), 4 deletions(-)
---
diff --git a/libsoup/soup-tld.c b/libsoup/soup-tld.c
index f598a805..f6e4db4b 100644
--- a/libsoup/soup-tld.c
+++ b/libsoup/soup-tld.c
@@ -209,6 +209,8 @@ soup_tld_get_base_domain_internal (const char *hostname, guint additional_domain
                char *orig_domain;
                gboolean domain_found;
                int *flags;
+               char *normalized_domain = NULL;
+               int domain_length;
 
                /* Valid hostnames neither start with a dot nor have more than one
                 * dot together.
@@ -222,7 +224,15 @@ soup_tld_get_base_domain_internal (const char *hostname, guint additional_domain
                }
 
                next_dot = strchr (cur_domain, '.');
-               domain_found = g_hash_table_lookup_extended (rules, cur_domain, (gpointer *) &orig_domain, 
(gpointer *) &flags);
+
+               /* Discard trailing dot if any before looking up. */
+               domain_length = strlen (cur_domain);
+               if (cur_domain[domain_length - 1] == '.')
+                       normalized_domain = g_strndup (cur_domain, domain_length - 1);
+               domain_found = g_hash_table_lookup_extended (rules, normalized_domain ? normalized_domain : 
cur_domain, (gpointer *) &orig_domain, (gpointer *) &flags);
+               g_free (normalized_domain);
+               normalized_domain = NULL;
+
                /* We compare the keys just to be sure that we haven't hit a collision */
                if (domain_found && !strncmp (orig_domain, cur_domain, strlen (orig_domain))) {
                        if (*flags & SOUP_TLD_RULE_MATCH_ALL) {
diff --git a/tests/cookies-test.c b/tests/cookies-test.c
index 8735964c..6bc228bb 100644
--- a/tests/cookies-test.c
+++ b/tests/cookies-test.c
@@ -101,6 +101,7 @@ do_cookies_subdomain_policy_test (void)
        GSList *cookies;
        SoupURI *uri1;
        SoupURI *uri2;
+       SoupURI *uri3;
 
        g_test_bug ("792130");
 
@@ -109,6 +110,7 @@ do_cookies_subdomain_policy_test (void)
         */
        uri1 = soup_uri_new ("https://www.gnome.org";);
        uri2 = soup_uri_new ("https://foundation.gnome.org";);
+       uri3 = soup_uri_new ("https://www.gnome.org.";);
 
        /* We can't check subdomains with a test server running on
         * localhost, so we'll just check the cookie jar API itself.
@@ -136,16 +138,74 @@ do_cookies_subdomain_policy_test (void)
        g_assert_cmpint (g_slist_length (cookies), ==, 2);
        g_slist_free_full (cookies, (GDestroyNotify)soup_cookie_free);
 
-       /* A leading dot in the domain property should not affect things.
-        * This cookie should be accepted. Three cookies in the jar.
+       /* Now some Domain attribute tests.*/
+       soup_cookie_jar_set_accept_policy (jar, SOUP_COOKIE_JAR_ACCEPT_ALWAYS);
+
+       /* The cookie must be rejected if the Domain is not an appropriate
+        * match for the URI. Still two cookies in the jar.
         */
-       soup_cookie_jar_set_cookie_with_first_party (jar, uri1, uri1, "4=foo; Domain=.www.gnome.org");
+       soup_cookie_jar_set_cookie (jar, uri1, "4=foo; Domain=gitlab.gnome.org");
+       cookies = soup_cookie_jar_all_cookies (jar);
+       g_assert_cmpint (g_slist_length (cookies), ==, 2);
+       g_slist_free_full (cookies, (GDestroyNotify)soup_cookie_free);
+
+       /* Now the Domain is an appropriate match. Three cookies in the jar. */
+       soup_cookie_jar_set_cookie (jar, uri1, "5=foo; Domain=gnome.org");
        cookies = soup_cookie_jar_all_cookies (jar);
        g_assert_cmpint (g_slist_length (cookies), ==, 3);
        g_slist_free_full (cookies, (GDestroyNotify)soup_cookie_free);
 
+       /* A leading dot in the domain property should not affect things.
+        * This cookie should be accepted. Four cookies in the jar.
+        */
+       soup_cookie_jar_set_cookie (jar, uri1, "6=foo; Domain=.www.gnome.org");
+       cookies = soup_cookie_jar_all_cookies (jar);
+       g_assert_cmpint (g_slist_length (cookies), ==, 4);
+       g_slist_free_full (cookies, (GDestroyNotify)soup_cookie_free);
+
+       /* The cookie must be rejected if the Domain ends in a trailing dot
+        * but the uri doesn't.
+        */
+       soup_cookie_jar_set_cookie (jar, uri1, "7=foo; Domain=www.gnome.org.");
+       cookies = soup_cookie_jar_all_cookies (jar);
+       g_assert_cmpint (g_slist_length (cookies), ==, 4);
+       g_slist_free_full (cookies, (GDestroyNotify)soup_cookie_free);
+
+       /* The cookie should be accepted if both Domain and URI end with a trailing
+        * dot and they are a match. Five cookies in the jar.
+        */
+       soup_cookie_jar_set_cookie (jar, uri3, "8=foo; Domain=gnome.org.");
+       cookies = soup_cookie_jar_all_cookies (jar);
+       g_assert_cmpint (g_slist_length (cookies), ==, 5);
+       g_slist_free_full (cookies, (GDestroyNotify)soup_cookie_free);
+
+       /* The cookie should be rejected if URI has trailing dot but Domain doesn't.
+        * Five cookies in the jar.
+        */
+       soup_cookie_jar_set_cookie (jar, uri3, "9=foo; Domain=gnome.org");
+       cookies = soup_cookie_jar_all_cookies (jar);
+       g_assert_cmpint (g_slist_length (cookies), ==, 5);
+       g_slist_free_full (cookies, (GDestroyNotify)soup_cookie_free);
+
+       /* It should not be possible to set a cookie for a TLD. Still five
+        * cookies in the jar.
+        */
+       soup_cookie_jar_set_cookie (jar, uri1, "10=foo; Domain=.org");
+       cookies = soup_cookie_jar_all_cookies (jar);
+       g_assert_cmpint (g_slist_length (cookies), ==, 5);
+       g_slist_free_full (cookies, (GDestroyNotify)soup_cookie_free);
+
+       /* It should still not be possible to set a cookie for a TLD, even if
+        * we are tricksy and have a trailing dot. Still only five cookies.
+        */
+       soup_cookie_jar_set_cookie (jar, uri3, "11=foo; Domain=.org.");
+       cookies = soup_cookie_jar_all_cookies (jar);
+       g_assert_cmpint (g_slist_length (cookies), ==, 5);
+       g_slist_free_full (cookies, (GDestroyNotify)soup_cookie_free);
+
        soup_uri_free (uri1);
        soup_uri_free (uri2);
+       soup_uri_free (uri3);
        g_object_unref (jar);
 }
 
diff --git a/tests/tld-test.c b/tests/tld-test.c
index 31cbb4b8..aae563ce 100644
--- a/tests/tld-test.c
+++ b/tests/tld-test.c
@@ -22,6 +22,9 @@ static struct {
        { ".example", NULL, SOUP_TLD_ERROR_INVALID_HOSTNAME },
        { ".example.com", NULL, SOUP_TLD_ERROR_INVALID_HOSTNAME },
        { ".example.example", NULL, SOUP_TLD_ERROR_INVALID_HOSTNAME },
+       /* Trailing dot. */
+       { ".com.", NULL, SOUP_TLD_ERROR_INVALID_HOSTNAME },
+       { "domain.biz.", "domain.biz.", -1 },
        /* TLD with only 1 rule. */
        { "biz", NULL, SOUP_TLD_ERROR_NOT_ENOUGH_DOMAINS },
        { "domain.biz", "domain.biz", -1 },