Bug 1440462 - Send httponly cookie names to content processes. r=jdm

Previously, if script tried to set a cookie that matched a cookie we had
received via Set-Cookie that was labeled httponly, script would think
that cookie was properly set (even though it wasn't). This ensures that
script knows just enough about httponly cookies to prevent this
inconsistent view while avoiding leakages of the potentially-sensitive
cookie values.

Differential Revision: https://phabricator.services.mozilla.com/D5700

--HG--
extra : moz-landing-system : lando

netwerk/cookie/CookieServiceChild.cpp

@@ -391,6 +391,11 @@ CookieServiceChild::GetCookieStringFromCookieHashTable(nsIURI                 *a
     if (!nsCookieService::DomainMatches(cookie, hostFromURI))
       continue;
 
+    // We don't show HttpOnly cookies in content processes.
+    if (cookie->IsHttpOnly()) {
+      continue;
+    }
+
     // if the cookie is secure and the host scheme isn't, we can't send it
     if (cookie->IsSecure() && !isSecure)
       continue;
@@ -521,9 +526,7 @@ CookieServiceChild::RecordDocumentCookie(nsCookie               *aCookie,
     return;
   }
 
-  if (!aCookie->IsHttpOnly()) {
-    cookiesList->AppendElement(aCookie);
-  }
+  cookiesList->AppendElement(aCookie);
 }
 
 nsresult
@@ -677,19 +680,38 @@ CookieServiceChild::SetCookieStringInternal(nsIURI *aHostURI,
     return NS_OK;
   }
 
+  nsCookieKey key(baseDomain, attrs);
+  CookiesList *cookies = mCookiesMap.Get(key);
+
   nsCString serverTimeString(aServerTime);
   int64_t serverTime = nsCookieService::ParseServerTime(serverTimeString);
   bool moreCookies;
   do {
     nsCookieAttributes cookieAttributes;
     bool canSetCookie = false;
-    nsCookieKey key(baseDomain, attrs);
     moreCookies = nsCookieService::CanSetCookie(aHostURI, key, cookieAttributes,
                                                 requireHostMatch, cookieStatus,
                                                 cookieString, serverTime, aFromHttp,
                                                 aChannel, mLeaveSecureAlone,
                                                 canSetCookie, mThirdPartyUtil);
 
+    // We need to see if the cookie we're setting would overwrite an httponly
+    // one. This would not affect anything we send over the net (those come from
+    // the parent, which already checks this), but script could see an
+    // inconsistent view of things.
+    if (cookies && canSetCookie && !aFromHttp) {
+      for (uint32_t i = 0; i < cookies->Length(); ++i) {
+        RefPtr<nsCookie> cookie = cookies->ElementAt(i);
+        if (cookie->Name().Equals(cookieAttributes.name) &&
+            cookie->Host().Equals(cookieAttributes.host) &&
+            cookie->Path().Equals(cookieAttributes.path) &&
+            cookie->IsHttpOnly()) {
+          // Can't overwrite an httponly cookie from a script context.
+          canSetCookie = false;
+        }
+      }
+    }
+
     if (canSetCookie) {
       SetCookieInternal(cookieAttributes, attrs, aChannel,
                         aFromHttp, permissionService);

netwerk/cookie/CookieServiceParent.cpp

@@ -100,10 +100,12 @@ CookieServiceParent::RemoveBatchDeletedCookies(nsIArray *aCookieList) {
     auto cookie = static_cast<nsCookie*>(xpcCookie.get());
     attrs = cookie->OriginAttributesRef();
     GetInfoFromCookie(cookie, cookieStruct);
-    if (!cookie->IsHttpOnly()) {
-      cookieStructList.AppendElement(cookieStruct);
-      attrsList.AppendElement(attrs);
+    if (cookie->IsHttpOnly()) {
+      // Child only needs to exist if an HttpOnly cookie exists, not its value
+      cookieStruct.value() = "";
     }
+    cookieStructList.AppendElement(cookieStruct);
+    attrsList.AppendElement(attrs);
   }
   Unused << SendRemoveBatchDeletedCookies(cookieStructList, attrsList);
 }
@@ -121,9 +123,10 @@ CookieServiceParent::RemoveCookie(nsICookie *aCookie)
   OriginAttributes attrs = cookie->OriginAttributesRef();
   CookieStruct cookieStruct;
   GetInfoFromCookie(cookie, cookieStruct);
-  if (!cookie->IsHttpOnly()) {
-    Unused << SendRemoveCookie(cookieStruct, attrs);
+  if (cookie->IsHttpOnly()) {
+    cookieStruct.value() = "";
   }
+  Unused << SendRemoveCookie(cookieStruct, attrs);
 }
 
 void
@@ -133,6 +136,9 @@ CookieServiceParent::AddCookie(nsICookie *aCookie)
   OriginAttributes attrs = cookie->OriginAttributesRef();
   CookieStruct cookieStruct;
   GetInfoFromCookie(cookie, cookieStruct);
+  if (cookie->IsHttpOnly()) {
+    cookieStruct.value() = "";
+  }
   Unused << SendAddCookie(cookieStruct, attrs);
 }
 
@@ -191,7 +197,9 @@ CookieServiceParent::SerialializeCookieList(const nsTArray<nsCookie*> &aFoundCoo
     nsCookie *cookie = aFoundCookieList.ElementAt(i);
     CookieStruct* cookieStruct = aCookiesList.AppendElement();
     cookieStruct->name() = cookie->Name();
-    cookieStruct->value() = cookie->Value();
+    if (!cookie->IsHttpOnly()) {
+      cookieStruct->value() = cookie->Value();
+    }
     cookieStruct->host() = cookie->Host();
     cookieStruct->path() = cookie->Path();
     cookieStruct->expiry() = cookie->Expiry();