Commit 6b11a0a0 authored by valenting's avatar valenting Committed by Richard Pospesel
Browse files

Bug 1779993 - Reject cookies with no name and a __Secure- or __Host- prefix... 

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
parent c70f1d68

netwerk/cookie/CookieService.cpp

+32 −1
Original line number Diff line number Diff line
@@ -1139,6 +1139,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,
@@ -1773,6 +1785,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,
@@ -1849,7 +1880,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

netwerk/cookie/CookieService.h

+1 −0
Original line number Diff line number Diff line
@@ -122,6 +122,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);

netwerk/test/gtest/TestCookie.cpp

+26 −0
Original line number Diff line number Diff line
@@ -1061,3 +1061,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));

}