cookie-jar: Reject cookies with control characters in name or value

3 files changed, 92 insertions, 4 deletions

diff --git a/libsoup/cookies/soup-cookie-jar.c b/libsoup/cookies/soup-cookie-jar.c
index f02a76c7..43f75046 100644
--- a/libsoup/cookies/soup-cookie-jar.c
+++ b/libsoup/cookies/soup-cookie-jar.c
@@ -564,6 +564,21 @@ incoming_cookie_is_third_party (SoupCookieJar            *jar,
         return retval;
 }
 
+static gboolean
+string_contains_ctrlcode (const char *s)
+{
+	const char *p;
+
+	p = s;
+	while (*p != '\0') {
+		if (g_ascii_iscntrl (*p) && *p != 0x09)
+			return TRUE;
+		
+		p++;
+	}
+	return FALSE;
+}
+
 /**
  * soup_cookie_jar_add_cookie_full:
  * @jar: a #SoupCookieJar
@@ -644,6 +659,22 @@ soup_cookie_jar_add_cookie_full (SoupCookieJar *jar, SoupCookie *cookie, GUri *u
 		}
 	}
 
+	/* Cookies should not take control characters %x00-1F / %x7F (defined by RFC 5234) in names or values,
+	 * with the exception of %x09 (the tab character).
+	 */    
+	const char *name, *value;
+	name = soup_cookie_get_name (cookie);
+	value = soup_cookie_get_value (cookie);
+	if (string_contains_ctrlcode (name) || string_contains_ctrlcode (value)) {
+		soup_cookie_free (cookie);
+		return;
+	}
+	
+	if (strlen(name) > 4096 || strlen(value) > 4096) {
+		soup_cookie_free (cookie);
+		return;
+	}
+	
         g_mutex_lock (&priv->mutex);
 
 	old_cookies = g_hash_table_lookup (priv->domains, soup_cookie_get_domain (cookie));
diff --git a/libsoup/cookies/soup-cookie.c b/libsoup/cookies/soup-cookie.c
index 0945d632..814d2b02 100644
--- a/libsoup/cookies/soup-cookie.c
+++ b/libsoup/cookies/soup-cookie.c
@@ -108,24 +108,32 @@ soup_cookie_domain_matches (SoupCookie *cookie, const char *host)
 	return soup_host_matches_host (cookie->domain, host);
 }
 
+#define IS_CNTRL(chr) ( g_ascii_iscntrl (chr) && chr != 0x09 )
+
 static inline const char *
 skip_lws (const char *s)
 {
-	while (g_ascii_isspace (*s))
+	while (g_ascii_isspace (*s)) {
+		if (IS_CNTRL (*s))
+			return s;
 		s++;
+	}
 	return s;
 }
 
 static inline const char *
 unskip_lws (const char *s, const char *start)
 {
-	while (s > start && g_ascii_isspace (*(s - 1)))
+	while (s > start && g_ascii_isspace (*(s - 1))) {
 		s--;
+		if (IS_CNTRL (*s))
+			return s;
+	}
 	return s;
 }
 
-#define is_attr_ender(ch) ((ch) < ' ' || (ch) == ';' || (ch) == ',' || (ch) == '=')
-#define is_value_ender(ch) ((ch) < ' ' || (ch) == ';')
+#define is_attr_ender(ch) ((ch) == '\0' || (ch) == ';' || (ch) == ',' || (ch) == '=')
+#define is_value_ender(ch) ((ch) == '\0' || (ch) == ';')
 
 static char *
 parse_value (const char **val_p, gboolean copy)
diff --git a/tests/cookies-test.c b/tests/cookies-test.c
index 91f6094e..2780e0ad 100644
--- a/tests/cookies-test.c
+++ b/tests/cookies-test.c
@@ -442,6 +442,54 @@ do_cookies_equal_nullpath (void)
 }
 
 static void
+do_cookies_parsing_control_characters (void)
+{
+	SoupCookieJar *jar;
+	GSList *cookies;
+	GUri *uri;
+	char buf[256];
+	int cntrl;
+
+	uri = g_uri_parse ("https://gnome.org", SOUP_HTTP_URI_FLAGS, NULL);
+	jar = soup_cookie_jar_new ();
+
+	/* Cookies should not take control characters %x00-1F / %x7F in names or values, 
+	 * with the exception of %x09 (the tab character). 
+	 */
+	for (cntrl = 0x01; cntrl <= 0x1F; cntrl++) {
+		if (cntrl == 0x09)
+			continue;
+
+		g_snprintf (buf, sizeof(buf), "name%c%x=value%x", cntrl, cntrl, cntrl);
+		soup_cookie_jar_set_cookie (jar, uri, buf);
+		g_snprintf (buf, sizeof(buf), "name%x=value%c%x", cntrl, cntrl, cntrl);
+		soup_cookie_jar_set_cookie (jar, uri, buf);
+
+		cookies = soup_cookie_jar_all_cookies (jar);
+		g_assert_cmpint (g_slist_length (cookies), ==,  0);
+		g_slist_free_full (cookies, (GDestroyNotify)soup_cookie_free);
+	}
+
+	cntrl = 0x7F;
+	g_snprintf (buf, sizeof(buf), "name%c%x=value%x", cntrl, cntrl, cntrl);
+	soup_cookie_jar_set_cookie (jar, uri, buf);
+	g_snprintf (buf, sizeof(buf), "name%x=value%c%x", cntrl, cntrl, cntrl);
+	soup_cookie_jar_set_cookie (jar, uri, buf);
+	cookies = soup_cookie_jar_all_cookies (jar);
+	g_assert_cmpint (g_slist_length (cookies), ==,  0);
+
+	/* Cookies are accepted with a tab (\t) in name or value. */
+	soup_cookie_jar_set_cookie (jar, uri, "name\x099=value9");
+	soup_cookie_jar_set_cookie (jar, uri, "name9=value\x099");
+	cookies = soup_cookie_jar_all_cookies (jar);
+	g_assert_cmpint (g_slist_length (cookies), ==,  2);
+
+	g_slist_free_full (cookies, (GDestroyNotify)soup_cookie_free);
+	g_uri_unref (uri);
+	g_object_unref (jar);
+}
+
+static void
 do_get_cookies_empty_host_test (void)
 {
 	SoupCookieJar *jar;
@@ -601,6 +649,7 @@ main (int argc, char **argv)
 	g_test_add_func ("/cookies/parsing", do_cookies_parsing_test);
 	g_test_add_func ("/cookies/parsing/no-path-null-origin", do_cookies_parsing_nopath_nullorigin);
 	g_test_add_func ("/cookies/parsing/equal-nullpath", do_cookies_equal_nullpath);
+	g_test_add_func ("/cookies/parsing/control-characters", do_cookies_parsing_control_characters);
 	g_test_add_func ("/cookies/get-cookies/empty-host", do_get_cookies_empty_host_test);
 	g_test_add_func ("/cookies/remove-feature", do_remove_feature_test);
 	g_test_add_func ("/cookies/secure-cookies", do_cookies_strict_secure_test);