From 5fbf64d96f02c248af50ad5ac56ea957bb763fcf Mon Sep 17 00:00:00 2001
From: Valentin Gosu <valentin.gosu@gmail.com>
Date: Wed, 7 Sep 2022 19:04:22 +0000
Subject: [PATCH] Bug 1779993 - Reject cookies with no name and a __Secure- or
 __Host- prefix r=necko-reviewers,kershaw a=RyanVM

Differential Revision: https://phabricator.services.mozilla.com/D156554
---
 netwerk/cookie/CookieService.cpp  | 33 ++++++++++++++++++++++++++++++-
 netwerk/cookie/CookieService.h    |  1 +
 netwerk/test/gtest/TestCookie.cpp | 26 ++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/netwerk/cookie/CookieService.cpp b/netwerk/cookie/CookieService.cpp
index b3fbe1c327cf9..9947a87ad89db 100644
--- a/netwerk/cookie/CookieService.cpp
+++ b/netwerk/cookie/CookieService.cpp
@@ -1207,6 +1207,18 @@ bool CookieService::CanSetCookie(
     return newCookie;
   }
 
+  if (!CheckHiddenPrefix(aCookieData)) {
+    COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, savedCookieHeader,
+                      "failed the CheckHiddenPrefix tests");
+    CookieLogging::LogMessageToConsole(
+        aCRC, aHostURI, nsIScriptError::warningFlag, CONSOLE_REJECTION_CATEGORY,
+        "CookieRejectedInvalidPrefix"_ns,
+        AutoTArray<nsString, 1>{
+            NS_ConvertUTF8toUTF16(aCookieData.name()),
+        });
+    return newCookie;
+  }
+
   // magic prefix checks. MUST be run after CheckDomain() and CheckPath()
   if (!CheckPrefixes(aCookieData, potentiallyTurstworthy)) {
     COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, savedCookieHeader,
@@ -1837,6 +1849,25 @@ bool CookieService::CheckDomain(CookieStruct& aCookieData, nsIURI* aHostURI,
   return true;
 }
 
+// static
+bool CookieService::CheckHiddenPrefix(CookieStruct& aCookieData) {
+  // If a cookie is nameless, then its value must not start with
+  // `__Host-` or `__Secure-`
+  if (!aCookieData.name().IsEmpty()) {
+    return true;
+  }
+
+  if (StringBeginsWith(aCookieData.value(), "__Host-"_ns)) {
+    return false;
+  }
+
+  if (StringBeginsWith(aCookieData.value(), "__Secure-"_ns)) {
+    return false;
+  }
+
+  return true;
+}
+
 namespace {
 nsAutoCString GetPathFromURI(nsIURI* aHostURI) {
   // strip down everything after the last slash to get the path,
@@ -1913,7 +1944,7 @@ bool CookieService::CheckPath(CookieStruct& aCookieData,
 // CheckPrefixes
 //
 // Reject cookies whose name starts with the magic prefixes from
-// https://tools.ietf.org/html/draft-ietf-httpbis-cookie-prefixes-00
+// https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis
 // if they do not meet the criteria required by the prefix.
 //
 // Must not be called until after CheckDomain() and CheckPath() have
diff --git a/netwerk/cookie/CookieService.h b/netwerk/cookie/CookieService.h
index caec5b785bef2..beab14b13b0c2 100644
--- a/netwerk/cookie/CookieService.h
+++ b/netwerk/cookie/CookieService.h
@@ -121,6 +121,7 @@ class CookieService final : public nsICookieService,
   static bool CheckDomain(CookieStruct& aCookieData, nsIURI* aHostURI,
                           const nsACString& aBaseDomain,
                           bool aRequireHostMatch);
+  static bool CheckHiddenPrefix(CookieStruct& aCookieData);
   static bool CheckPath(CookieStruct& aCookieData,
                         nsIConsoleReportCollector* aCRC, nsIURI* aHostURI);
   static bool CheckPrefixes(CookieStruct& aCookieData, bool aSecureRequest);
diff --git a/netwerk/test/gtest/TestCookie.cpp b/netwerk/test/gtest/TestCookie.cpp
index f65dc6ef92124..f3e1617b3c175 100644
--- a/netwerk/test/gtest/TestCookie.cpp
+++ b/netwerk/test/gtest/TestCookie.cpp
@@ -1057,3 +1057,29 @@ TEST(TestCookie, OnionSite)
   GetACookieNoHttp(cookieService, "http://123456789abcdef.onion/", cookie);
   EXPECT_TRUE(CheckResult(cookie.get(), MUST_EQUAL, "test=onion-security4"));
 }
+
+TEST(TestCookie, HiddenPrefix)
+{
+  nsresult rv;
+  nsCString cookie;
+
+  nsCOMPtr<nsICookieService> cookieService =
+      do_GetService(kCookieServiceCID, &rv);
+  ASSERT_TRUE(NS_SUCCEEDED(rv));
+
+  SetACookie(cookieService, "http://hiddenprefix.test/", "=__Host-test=a");
+  GetACookie(cookieService, "http://hiddenprefix.test/", cookie);
+  EXPECT_TRUE(CheckResult(cookie.get(), MUST_BE_NULL));
+
+  SetACookie(cookieService, "http://hiddenprefix.test/", "=__Secure-test=a");
+  GetACookie(cookieService, "http://hiddenprefix.test/", cookie);
+  EXPECT_TRUE(CheckResult(cookie.get(), MUST_BE_NULL));
+
+  SetACookie(cookieService, "http://hiddenprefix.test/", "=__Host-check");
+  GetACookie(cookieService, "http://hiddenprefix.test/", cookie);
+  EXPECT_TRUE(CheckResult(cookie.get(), MUST_BE_NULL));
+
+  SetACookie(cookieService, "http://hiddenprefix.test/", "=__Secure-check");
+  GetACookie(cookieService, "http://hiddenprefix.test/", cookie);
+  EXPECT_TRUE(CheckResult(cookie.get(), MUST_BE_NULL));
+}