Bug 1191423 - Disallow illegal characters in cookies set via HTTP. r=jduell, a=sylvestre
authorNicholas Hurley <hurley@todesschaf.org>
Fri, 04 Sep 2015 09:58:31 -0700
changeset 296176 d0a3f5e5adc0dd84e1ddce9451b3ce36f997e897
parent 296175 a73cb82d3db1a51946e753837381731cc2a9110e
child 296177 1e921bf3f7049279944f832b99a42b4170fce08a
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjduell, sylvestre
bugs1191423
milestone43.0a2
Bug 1191423 - Disallow illegal characters in cookies set via HTTP. r=jduell, a=sylvestre
netwerk/cookie/nsCookieService.cpp
netwerk/test/unit/test_cookie_blacklist.js
netwerk/test/unit/xpcshell.ini
--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@@ -2878,16 +2878,33 @@ nsCookieService::SetCookieInternal(nsIUR
     COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, savedCookieHeader, "failed the domain tests");
     return newCookie;
   }
   if (!CheckPath(cookieAttributes, aHostURI)) {
     COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, savedCookieHeader, "failed the path tests");
     return newCookie;
   }
 
+  // reject cookie if value contains an RFC 6265 disallowed character - see
+  // https://bugzilla.mozilla.org/show_bug.cgi?id=1191423
+  // NOTE: this is not the full set of characters disallowed by 6265 - notably
+  // 0x09, 0x20, 0x22, 0x2C, 0x5C, and 0x7F are missing from this list. This is
+  // for parity with Chrome. This only applies to cookies set via the Set-Cookie
+  // header, as document.cookie is defined to be UTF-8. Hooray for
+  // symmetry!</sarcasm>
+  const char illegalCharacters[] = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+                                     0x08, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F,
+                                     0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16,
+                                     0x17, 0x18, 0x19, 0x1A, 0x1B, 0x1C, 0x1D,
+                                     0x1E, 0x1F, 0x3B, 0x00 };
+  if (aFromHttp && (cookieAttributes.value.FindCharInSet(illegalCharacters, 0) != -1)) {
+    COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, savedCookieHeader, "invalid value character");
+    return newCookie;
+  }
+
   // create a new nsCookie and copy attributes
   nsRefPtr<nsCookie> cookie =
     nsCookie::Create(cookieAttributes.name,
                      cookieAttributes.value,
                      cookieAttributes.host,
                      cookieAttributes.path,
                      cookieAttributes.expiryTime,
                      currentTimeInUsec,
new file mode 100644
--- /dev/null
+++ b/netwerk/test/unit/test_cookie_blacklist.js
@@ -0,0 +1,16 @@
+const GOOD_COOKIE = "GoodCookie=OMNOMNOM";
+
+function run_test() {
+  var ios = Cc["@mozilla.org/network/io-service;1"].getService(Ci.nsIIOService);
+  var cookieURI = ios.newURI("http://mozilla.org/test_cookie_blacklist.js",
+                             null, null);
+
+  var cookieService = Cc["@mozilla.org/cookieService;1"]
+                        .getService(Ci.nsICookieService);
+  cookieService.setCookieStringFromHttp(cookieURI, cookieURI, null, "BadCookie1=\x01", null, null);
+  cookieService.setCookieStringFromHttp(cookieURI, cookieURI, null, "BadCookie2=\v", null, null);
+  cookieService.setCookieStringFromHttp(cookieURI, cookieURI, null, GOOD_COOKIE, null, null);
+
+  var storedCookie = cookieService.getCookieString(cookieURI, null);
+  do_check_eq(storedCookie, GOOD_COOKIE);
+}
--- a/netwerk/test/unit/xpcshell.ini
+++ b/netwerk/test/unit/xpcshell.ini
@@ -324,8 +324,9 @@ skip-if = os == "android"
 [test_packaged_app_service.js]
 [test_packaged_app_verifier.js]
 [test_suspend_channel_before_connect.js]
 [test_inhibit_caching.js]
 [test_dns_disable_ipv4.js]
 [test_dns_disable_ipv6.js]
 [test_packaged_app_service_paths.js]
 [test_bug1195415.js]
+[test_cookie_blacklist.js]