From 462536e113841a062ebb64bef31896b960b37e65 Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Wed, 5 Oct 2022 17:36:16 -0500 Subject: [System:PlatformPrefs] Update list of external protocol handlers --- system/platform-prefs.js | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/system/platform-prefs.js b/system/platform-prefs.js index 327ef332b..16ab9201c 100644 --- a/system/platform-prefs.js +++ b/system/platform-prefs.js @@ -1194,19 +1194,24 @@ pref("network.protocol-handler.external-default", true); // OK to load pref("network.protocol-handler.warn-external-default", true); // warn before load // Prevent using external protocol handlers for these schemes +pref("network.protocol-handler.external.afp", false); +pref("network.protocol-handler.external.data", false); +pref("network.protocol-handler.external.disk", false); +pref("network.protocol-handler.external.disks", false); pref("network.protocol-handler.external.hcp", false); -pref("network.protocol-handler.external.vbscript", false); pref("network.protocol-handler.external.javascript", false); -pref("network.protocol-handler.external.data", false); -pref("network.protocol-handler.external.ms-help", false); pref("network.protocol-handler.external.mk", false); +pref("network.protocol-handler.external.moz-icon", false); pref("network.protocol-handler.external.res", false); pref("network.protocol-handler.external.shell", false); +pref("network.protocol-handler.external.vbscript", false); pref("network.protocol-handler.external.vnd.ms.radio", false); -pref("network.protocol-handler.external.disk", false); -pref("network.protocol-handler.external.disks", false); -pref("network.protocol-handler.external.afp", false); -pref("network.protocol-handler.external.moz-icon", false); +#ifdef XP_WIN +pref("network.protocol-handler.external.ms-help", false); +pref("network.protocol-handler.external.ms-msdt", false); +pref("network.protocol-handler.external.search", false); +pref("network.protocol-handler.external.search-ms", false); +#endif // Don't allow external protocol handlers for common typos pref("network.protocol-handler.external.ttp", false); // http -- cgit v1.2.3 From dad4dab99722267de87d9c6b5fda34145c6361e7 Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Wed, 5 Oct 2022 17:39:13 -0500 Subject: [DOM:Media] Remove potentially unsafe type accesses when debug logging --- dom/media/mediasource/TrackBuffersManager.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dom/media/mediasource/TrackBuffersManager.cpp b/dom/media/mediasource/TrackBuffersManager.cpp index da21e0b39..907ee40e8 100644 --- a/dom/media/mediasource/TrackBuffersManager.cpp +++ b/dom/media/mediasource/TrackBuffersManager.cpp @@ -22,15 +22,15 @@ extern mozilla::LogModule* GetMediaSourceLog(); -#define MSE_DEBUG(arg, ...) MOZ_LOG(GetMediaSourceLog(), mozilla::LogLevel::Debug, ("TrackBuffersManager(%p:%s)::%s: " arg, this, mType.get(), __func__, ##__VA_ARGS__)) -#define MSE_DEBUGV(arg, ...) MOZ_LOG(GetMediaSourceLog(), mozilla::LogLevel::Verbose, ("TrackBuffersManager(%p:%s)::%s: " arg, this, mType.get(), __func__, ##__VA_ARGS__)) +#define MSE_DEBUG(arg, ...) MOZ_LOG(GetMediaSourceLog(), mozilla::LogLevel::Debug, ("TrackBuffersManager(%p)::%s: " arg, this,__func__, ##__VA_ARGS__)) +#define MSE_DEBUGV(arg, ...) MOZ_LOG(GetMediaSourceLog(), mozilla::LogLevel::Verbose, ("TrackBuffersManager(%p)::%s: " arg, this, __func__, ##__VA_ARGS__)) mozilla::LogModule* GetMediaSourceSamplesLog() { static mozilla::LazyLogModule sLogModule("MediaSourceSamples"); return sLogModule; } -#define SAMPLE_DEBUG(arg, ...) MOZ_LOG(GetMediaSourceSamplesLog(), mozilla::LogLevel::Debug, ("TrackBuffersManager(%p:%s)::%s: " arg, this, mType.get(), __func__, ##__VA_ARGS__)) +#define SAMPLE_DEBUG(arg, ...) MOZ_LOG(GetMediaSourceSamplesLog(), mozilla::LogLevel::Debug, ("TrackBuffersManager(%p)::%s: " arg, this, __func__, ##__VA_ARGS__)) namespace mozilla { -- cgit v1.2.3 From 0dc28df4dbd3549fb0922996036d95fce1cf90cf Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Wed, 5 Oct 2022 17:41:33 -0500 Subject: [System:Network] Clear PAC loader when the load failed. --- system/network/base/nsPACMan.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/system/network/base/nsPACMan.cpp b/system/network/base/nsPACMan.cpp index 37d3e8b6b..f00c3cc43 100644 --- a/system/network/base/nsPACMan.cpp +++ b/system/network/base/nsPACMan.cpp @@ -474,6 +474,11 @@ nsPACMan::StartLoading() void nsPACMan::OnLoadFailure() { + // We have to clear the loader to indicate that we are currently not loading PAC. + if (mLoader) { + mLoader = nullptr; + } + int32_t minInterval = 5; // 5 seconds int32_t maxInterval = 300; // 5 minutes -- cgit v1.2.3 From 7e0d94a048cb7a73af5638f46bdb65794bcc4292 Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Wed, 5 Oct 2022 17:42:46 -0500 Subject: [DOM:Base] Use the sanitizer to restrict href in svg:use to fragment-only URLs --- dom/base/nsTreeSanitizer.cpp | 18 +++++++++++++++--- dom/base/nsTreeSanitizer.h | 4 +++- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/dom/base/nsTreeSanitizer.cpp b/dom/base/nsTreeSanitizer.cpp index 39c2408b7..970e4386b 100644 --- a/dom/base/nsTreeSanitizer.cpp +++ b/dom/base/nsTreeSanitizer.cpp @@ -1185,7 +1185,8 @@ nsTreeSanitizer::SanitizeAttributes(mozilla::dom::Element* aElement, continue; } if (IsURL(aURLs, attrLocal)) { - if (SanitizeURL(aElement, attrNs, attrLocal)) { + bool fragmentOnly = aElement->IsSVGElement(nsGkAtoms::use); + if (SanitizeURL(aElement, attrNs, attrLocal, fragmentOnly)) { // in case the attribute removal shuffled the attribute order, start // the loop again. --ac; @@ -1239,7 +1240,8 @@ nsTreeSanitizer::SanitizeAttributes(mozilla::dom::Element* aElement, // else not allowed } else if (aAllowXLink && kNameSpaceID_XLink == attrNs) { if (nsGkAtoms::href == attrLocal) { - if (SanitizeURL(aElement, attrNs, attrLocal)) { + bool fragmentOnly = aElement->IsSVGElement(nsGkAtoms::use); + if (SanitizeURL(aElement, attrNs, attrLocal, fragmentOnly)) { // in case the attribute removal shuffled the attribute order, start // the loop again. --ac; @@ -1273,7 +1275,8 @@ nsTreeSanitizer::SanitizeAttributes(mozilla::dom::Element* aElement, bool nsTreeSanitizer::SanitizeURL(mozilla::dom::Element* aElement, int32_t aNamespace, - nsIAtom* aLocalName) + nsIAtom* aLocalName, + bool aFragmentOnly) { nsAutoString value; aElement->GetAttr(aNamespace, aLocalName, value); @@ -1282,6 +1285,15 @@ nsTreeSanitizer::SanitizeURL(mozilla::dom::Element* aElement, static const char* kWhitespace = "\n\r\t\b"; const nsAString& v = nsContentUtils::TrimCharsInSet(kWhitespace, value); + // Fragment-only url cannot be harmful. + if (!v.IsEmpty() && v.First() == u'#') { + return false; + } + // if we allow only same-document fragment URLs, stop and remove here + if (aFragmentOnly) { + aElement->UnsetAttr(aNamespace, aLocalName, false); + return true; + } nsIScriptSecurityManager* secMan = nsContentUtils::GetSecurityManager(); uint32_t flags = nsIScriptSecurityManager::DISALLOW_INHERIT_PRINCIPAL; diff --git a/dom/base/nsTreeSanitizer.h b/dom/base/nsTreeSanitizer.h index b4a333f61..fe4917150 100644 --- a/dom/base/nsTreeSanitizer.h +++ b/dom/base/nsTreeSanitizer.h @@ -143,11 +143,13 @@ class MOZ_STACK_CLASS nsTreeSanitizer { * @param aElement the element whose attribute to possibly modify * @param aNamespace the namespace of the URL attribute * @param aLocalName the local name of the URL attribute + * @param aFragmentOnly allows same-document references only * @return true if the attribute was removed and false otherwise */ bool SanitizeURL(mozilla::dom::Element* aElement, int32_t aNamespace, - nsIAtom* aLocalName); + nsIAtom* aLocalName, + bool aFragmentOnly = false); /** * Checks a style rule for the presence of the 'binding' CSS property and -- cgit v1.2.3 From 875f8916352cda572a764f39eea533e27241796f Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Wed, 5 Oct 2022 17:43:48 -0500 Subject: [XPCOM:Glue] Crash safely when TArray replacements are OOB. In the unlikely event of TArray element replacement calls are OOB, crash safely with a debug breakpoint instead of corrupting memory. --- xpcom/glue/nsTArray.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/xpcom/glue/nsTArray.h b/xpcom/glue/nsTArray.h index 22d6ab7b3..03913a376 100644 --- a/xpcom/glue/nsTArray.h +++ b/xpcom/glue/nsTArray.h @@ -2018,6 +2018,12 @@ auto nsTArray_Impl::ReplaceElementsAt(index_type aStart, size_type aCount, const Item* aArray, size_type aArrayLen) -> elem_type* { + if (MOZ_UNLIKELY(aStart > Length())) { + InvalidArrayIndex_CRASH(aStart, Length()); + } + if (MOZ_UNLIKELY(aCount > Length() - aStart)) { + InvalidArrayIndex_CRASH(aStart + aCount, Length()); + } // Adjust memory allocation up-front to catch errors. if (!ActualAlloc::Successful(this->template EnsureCapacity( Length() + aArrayLen - aCount, sizeof(elem_type)))) { -- cgit v1.2.3 From 82f0220e74d5882ac826cbd0faf9dd4157e8caca Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Wed, 5 Oct 2022 17:45:02 -0500 Subject: [DOM:Perf] Add extra check for performance API Next Hop protocol. --- dom/performance/PerformanceResourceTiming.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dom/performance/PerformanceResourceTiming.h b/dom/performance/PerformanceResourceTiming.h index b4775d432..63a8c2414 100644 --- a/dom/performance/PerformanceResourceTiming.h +++ b/dom/performance/PerformanceResourceTiming.h @@ -54,7 +54,9 @@ public: void GetNextHopProtocol(nsAString& aNextHopProtocol) const { - aNextHopProtocol = mNextHopProtocol; + if (mTiming && mTiming->TimingAllowed()) { + aNextHopProtocol = mNextHopProtocol; + } } void SetNextHopProtocol(const nsAString& aNextHopProtocol) -- cgit v1.2.3 From dc5571eefd8247b14edbee33c8b881d6b9e97e96 Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Wed, 5 Oct 2022 17:59:56 -0500 Subject: [Multi:Sec] Be more explicit about CSP checks and reports. --- components/htmlfive/nsHtml5TreeOpExecutor.cpp | 56 +++++++++++++++++++++- dom/html/HTMLFormElement.cpp | 3 +- dom/html/HTMLSharedElement.cpp | 3 +- system/security/content/nsCSPContext.cpp | 7 +-- .../security/content/nsIContentSecurityPolicy.idl | 11 +++-- 5 files changed, 70 insertions(+), 10 deletions(-) diff --git a/components/htmlfive/nsHtml5TreeOpExecutor.cpp b/components/htmlfive/nsHtml5TreeOpExecutor.cpp index 8cf22d804..c79af5321 100644 --- a/components/htmlfive/nsHtml5TreeOpExecutor.cpp +++ b/components/htmlfive/nsHtml5TreeOpExecutor.cpp @@ -1019,9 +1019,63 @@ nsHtml5TreeOpExecutor::SetSpeculationBase(const nsAString& aURL) return; } const nsCString& charset = mDocument->GetDocumentCharacterSet(); - DebugOnly rv = NS_NewURI(getter_AddRefs(mSpeculationBaseURI), aURL, + nsCOMPtr newBaseURI; + nsresult rv = NS_NewURI(getter_AddRefs(newBaseURI), aURL, charset.get(), mDocument->GetDocumentURI()); NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Failed to create a URI"); + if (!newBaseURI) { + return; + } + + if (!CSPService::sCSPEnabled) { + // If CSP is not enabled, just pass back the URI + mSpeculationBaseURI = newBaseURI; + return; + } + + NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); + + nsCOMPtr principal = mDocument->NodePrincipal(); + nsCOMPtr domDoc = do_QueryInterface(mDocument); + + // Check the document's CSP usually delivered via the CSP header. + nsCOMPtr documentCsp; + rv = principal->EnsureCSP(domDoc, getter_AddRefs(documentCsp)); + NS_ENSURE_SUCCESS_VOID(rv); + if (documentCsp) { + // base-uri should not fallback to the default-src and preloads should not + // trigger violation reports. + bool cspPermitsBaseURI = true; + rv = documentCsp->Permits( + newBaseURI, + nsIContentSecurityPolicy::BASE_URI_DIRECTIVE, + true /* aSpecific */, + false /* aSendViolationReports */, + &cspPermitsBaseURI); + if (NS_FAILED(rv) || !cspPermitsBaseURI) { + return; + } + } + + // Also check the CSP discovered from the tag during speculative + // parsing. + nsCOMPtr preloadCsp; + rv = principal->EnsurePreloadCSP(domDoc, getter_AddRefs(preloadCsp)); + NS_ENSURE_SUCCESS_VOID(rv); + if (preloadCsp) { + bool cspPermitsBaseURI = true; + rv = preloadCsp->Permits( + newBaseURI, + nsIContentSecurityPolicy::BASE_URI_DIRECTIVE, + true /* aSpecific */, + false /* aSendViolationReports */, + &cspPermitsBaseURI); + if (NS_FAILED(rv) || !cspPermitsBaseURI) { + return; + } + } + + mSpeculationBaseURI = newBaseURI; } void diff --git a/dom/html/HTMLFormElement.cpp b/dom/html/HTMLFormElement.cpp index 2fe452bcd..c8bb52637 100644 --- a/dom/html/HTMLFormElement.cpp +++ b/dom/html/HTMLFormElement.cpp @@ -1721,7 +1721,8 @@ HTMLFormElement::GetActionURL(nsIURI** aActionURL, // policy - do *not* consult default-src, see: // http://www.w3.org/TR/CSP2/#directive-default-src rv = csp->Permits(actionURL, nsIContentSecurityPolicy::FORM_ACTION_DIRECTIVE, - true, &permitsFormAction); + true /*aSpecific */, true /* aSendViolationReports */, + &permitsFormAction); NS_ENSURE_SUCCESS(rv, rv); if (!permitsFormAction) { return NS_ERROR_CSP_FORM_ACTION_VIOLATION; diff --git a/dom/html/HTMLSharedElement.cpp b/dom/html/HTMLSharedElement.cpp index e8c75f8aa..90f9ff62e 100644 --- a/dom/html/HTMLSharedElement.cpp +++ b/dom/html/HTMLSharedElement.cpp @@ -191,7 +191,8 @@ SetBaseURIUsingFirstBaseWithHref(nsIDocument* aDocument, nsIContent* aMustMatch) // http://www.w3.org/TR/CSP2/#directive-default-src bool cspPermitsBaseURI = true; rv = csp->Permits(newBaseURI, nsIContentSecurityPolicy::BASE_URI_DIRECTIVE, - true, &cspPermitsBaseURI); + true /* aSpecific */, true /* aSendViolationReports */, + &cspPermitsBaseURI); if (NS_FAILED(rv) || !cspPermitsBaseURI) { newBaseURI = nullptr; } diff --git a/system/security/content/nsCSPContext.cpp b/system/security/content/nsCSPContext.cpp index ec698b91d..544fe118b 100644 --- a/system/security/content/nsCSPContext.cpp +++ b/system/security/content/nsCSPContext.cpp @@ -1309,6 +1309,7 @@ NS_IMETHODIMP nsCSPContext::Permits(nsIURI* aURI, CSPDirective aDir, bool aSpecific, + bool aSendViolationReports, bool* outPermits) { // Can't perform check without aURI @@ -1323,13 +1324,13 @@ nsCSPContext::Permits(nsIURI* aURI, false, // not redirected. false, // not a preload. aSpecific, - true, // send violation reports + aSendViolationReports, true, // send blocked URI in violation reports false); // not parser created if (CSPCONTEXTLOGENABLED()) { - CSPCONTEXTLOG(("nsCSPContext::Permits, aUri: %s, aDir: %d, isAllowed: %s", - aURI->GetSpecOrDefault().get(), aDir, + CSPCONTEXTLOG(("nsCSPContext::Permits, aUri: %s, aDir: %s, isAllowed: %s", + aURI->GetSpecOrDefault().get(), CSP_CSPDirectiveToString(aDir), *outPermits ? "allow" : "deny")); } diff --git a/system/security/content/nsIContentSecurityPolicy.idl b/system/security/content/nsIContentSecurityPolicy.idl index da4297f33..e76c39c44 100644 --- a/system/security/content/nsIContentSecurityPolicy.idl +++ b/system/security/content/nsIContentSecurityPolicy.idl @@ -252,9 +252,6 @@ interface nsIContentSecurityPolicy : nsISerializable /** * Checks if a specific directive permits loading of a URI. * - * NOTE: Calls to this may trigger violation reports when queried, so the - * return value should not be cached. - * * @param aURI * The URI about to be loaded or used. * @param aDir @@ -266,11 +263,17 @@ interface nsIContentSecurityPolicy : nsISerializable * "false" allows CSP to fall back to default-src. This function * behaves the same for both values of canUseDefault when querying * directives that don't fall-back. + * @param aSendViolationReports + * If `true` and the uri is not allowed then trigger violation reports. + * This should be `false` for caching or preloads. * @return * Whether or not the provided URI is allowed by CSP under the given * directive. (block the pending operation if false). */ - boolean permits(in nsIURI aURI, in CSPDirective aDir, in boolean aSpecific); + boolean permits(in nsIURI aURI, + in CSPDirective aDir, + in boolean aSpecific, + in boolean aSendViolationReports); /** * Delegate method called by the service when sub-elements of the protected -- cgit v1.2.3 From 14f8e3e2e95e86a995627add57bf71de7c7edcaa Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Wed, 5 Oct 2022 18:03:23 -0500 Subject: [System:Network] Reject cookies with no name and a __Secure- or __Host- prefix --- system/network/cookie/nsCookieService.cpp | 87 ++++++++++++++++++++----------- system/network/cookie/nsCookieService.h | 1 + 2 files changed, 58 insertions(+), 30 deletions(-) diff --git a/system/network/cookie/nsCookieService.cpp b/system/network/cookie/nsCookieService.cpp index 9a08bd1d7..828b8920c 100644 --- a/system/network/cookie/nsCookieService.cpp +++ b/system/network/cookie/nsCookieService.cpp @@ -3344,6 +3344,10 @@ nsCookieService::SetCookieInternal(nsIURI *aHostURI, COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, savedCookieHeader, "failed the path tests"); return newCookie; } + if (!CheckHiddenPrefix(cookieAttributes)) { + COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, savedCookieHeader, "failed the CheckHiddenPrefix tests"); + return newCookie; + } // magic prefix checks. MUST be run after CheckDomain() and CheckPath() if (!CheckPrefixes(cookieAttributes, isHTTPS)) { COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, savedCookieHeader, "failed the prefix tests"); @@ -4042,7 +4046,7 @@ nsCookieService::CheckPrefs(nsIURI *aHostURI, // processes domain attribute, and returns true if host has permission to set for this domain. bool -nsCookieService::CheckDomain(nsCookieAttributes &aCookieAttributes, +nsCookieService::CheckDomain(nsCookieAttributes &aCookie, nsIURI *aHostURI, const nsCString &aBaseDomain, bool aRequireHostMatch) @@ -4057,15 +4061,15 @@ nsCookieService::CheckDomain(nsCookieAttributes &aCookieAttributes, aHostURI->GetAsciiHost(hostFromURI); // if a domain is given, check the host has permission - if (!aCookieAttributes.host.IsEmpty()) { + if (!aCookie.host.IsEmpty()) { // Tolerate leading '.' characters, but not if it's otherwise an empty host. - if (aCookieAttributes.host.Length() > 1 && - aCookieAttributes.host.First() == '.') { - aCookieAttributes.host.Cut(0, 1); + if (aCookie.host.Length() > 1 && + aCookie.host.First() == '.') { + aCookie.host.Cut(0, 1); } // switch to lowercase now, to avoid case-insensitive compares everywhere - ToLowerCase(aCookieAttributes.host); + ToLowerCase(aCookie.host); // check whether the host is either an IP address, an alias such as // 'localhost', an eTLD such as 'co.uk', or the empty string. in these @@ -4073,14 +4077,14 @@ nsCookieService::CheckDomain(nsCookieAttributes &aCookieAttributes, // as a non-domain one. bug 105917 originally noted the requirement to deal // with IP addresses. if (aRequireHostMatch) - return hostFromURI.Equals(aCookieAttributes.host); + return hostFromURI.Equals(aCookie.host); // ensure the proposed domain is derived from the base domain; and also // that the host domain is derived from the proposed domain (per RFC2109). - if (IsSubdomainOf(aCookieAttributes.host, aBaseDomain) && - IsSubdomainOf(hostFromURI, aCookieAttributes.host)) { + if (IsSubdomainOf(aCookie.host, aBaseDomain) && + IsSubdomainOf(hostFromURI, aCookie.host)) { // prepend a dot to indicate a domain cookie - aCookieAttributes.host.Insert(NS_LITERAL_CSTRING("."), 0); + aCookie.host.Insert(NS_LITERAL_CSTRING("."), 0); return true; } @@ -4095,7 +4099,7 @@ nsCookieService::CheckDomain(nsCookieAttributes &aCookieAttributes, } // no domain specified, use hostFromURI - aCookieAttributes.host = hostFromURI; + aCookie.host = hostFromURI; return true; } @@ -4121,12 +4125,12 @@ GetPathFromURI(nsIURI* aHostURI) } bool -nsCookieService::CheckPath(nsCookieAttributes &aCookieAttributes, +nsCookieService::CheckPath(nsCookieAttributes &aCookie, nsIURI *aHostURI) { // if a path is given, check the host has permission - if (aCookieAttributes.path.IsEmpty() || aCookieAttributes.path.First() != '/') { - aCookieAttributes.path = GetPathFromURI(aHostURI); + if (aCookie.path.IsEmpty() || aCookie.path.First() != '/') { + aCookie.path = GetPathFromURI(aHostURI); #if 0 } else { @@ -4139,19 +4143,42 @@ nsCookieService::CheckPath(nsCookieAttributes &aCookieAttributes, // get path from aHostURI nsAutoCString pathFromURI; if (NS_FAILED(aHostURI->GetPath(pathFromURI)) || - !StringBeginsWith(pathFromURI, aCookieAttributes.path)) { + !StringBeginsWith(pathFromURI, aCookie.path)) { return false; } #endif } - if (aCookieAttributes.path.Length() > kMaxBytesPerPath || - aCookieAttributes.path.Contains('\t')) + if (aCookie.path.Length() > kMaxBytesPerPath || + aCookie.path.Contains('\t')) return false; return true; } +bool +nsCookieService::CheckHiddenPrefix(nsCookieAttributes &aCookie) { + // If a cookie is nameless, then its value must not start with + // `__Host-` or `__Secure-` + if (aCookie.name.Length() != 0) { + return true; + } + + static const char kSecure[] = "__Secure-"; + static const char kHost[] = "__Host-"; + static const int kSecureLen = sizeof( kSecure ) - 1; + static const int kHostLen = sizeof( kHost ) - 1; + + bool isSecure = strncmp( aCookie.value.get(), kSecure, kSecureLen ) == 0; + bool isHost = strncmp( aCookie.value.get(), kHost, kHostLen ) == 0; + + if (isSecure || isHost) { + return false; + } + + return true; +} + // CheckPrefixes // // Reject cookies whose name starts with the magic prefixes from @@ -4161,7 +4188,7 @@ nsCookieService::CheckPath(nsCookieAttributes &aCookieAttributes, // Must not be called until after CheckDomain() and CheckPath() have // regularized and validated the nsCookieAttributes values! bool -nsCookieService::CheckPrefixes(nsCookieAttributes &aCookieAttributes, +nsCookieService::CheckPrefixes(nsCookieAttributes &aCookie, bool aSecureRequest) { static const char kSecure[] = "__Secure-"; @@ -4169,15 +4196,15 @@ nsCookieService::CheckPrefixes(nsCookieAttributes &aCookieAttributes, static const int kSecureLen = sizeof( kSecure ) - 1; static const int kHostLen = sizeof( kHost ) - 1; - bool isSecure = strncmp( aCookieAttributes.name.get(), kSecure, kSecureLen ) == 0; - bool isHost = strncmp( aCookieAttributes.name.get(), kHost, kHostLen ) == 0; + bool isSecure = strncmp( aCookie.value.get(), kSecure, kSecureLen ) == 0; + bool isHost = strncmp( aCookie.value.get(), kHost, kHostLen ) == 0; if ( !isSecure && !isHost ) { // not one of the magic prefixes: carry on return true; } - if ( !aSecureRequest || !aCookieAttributes.isSecure ) { + if ( !aSecureRequest || !aCookie.isSecure ) { // the magic prefixes may only be used from a secure request and // the secure attribute must be set on the cookie return false; @@ -4190,8 +4217,8 @@ nsCookieService::CheckPrefixes(nsCookieAttributes &aCookieAttributes, // them. In particular all explicit domain attributes result in a host // that starts with a dot, and if the host doesn't start with a dot it // correctly matches the true host. - if ( aCookieAttributes.host[0] == '.' || - !aCookieAttributes.path.EqualsLiteral( "/" )) { + if ( aCookie.host[0] == '.' || + !aCookie.path.EqualsLiteral( "/" )) { return false; } } @@ -4200,7 +4227,7 @@ nsCookieService::CheckPrefixes(nsCookieAttributes &aCookieAttributes, } bool -nsCookieService::GetExpiry(nsCookieAttributes &aCookieAttributes, +nsCookieService::GetExpiry(nsCookieAttributes &aCookie, int64_t aServerTime, int64_t aCurrentTime) { @@ -4212,10 +4239,10 @@ nsCookieService::GetExpiry(nsCookieAttributes &aCookieAttributes, * Note: We need to consider accounting for network lag here, per RFC. */ // check for max-age attribute first; this overrides expires attribute - if (!aCookieAttributes.maxage.IsEmpty()) { + if (!aCookie.maxage.IsEmpty()) { // obtain numeric value of maxageAttribute int64_t maxage; - int32_t numInts = PR_sscanf(aCookieAttributes.maxage.get(), "%lld", &maxage); + int32_t numInts = PR_sscanf(aCookie.maxage.get(), "%lld", &maxage); // default to session cookie if the conversion failed if (numInts != 1) { @@ -4224,14 +4251,14 @@ nsCookieService::GetExpiry(nsCookieAttributes &aCookieAttributes, // if this addition overflows, expiryTime will be less than currentTime // and the cookie will be expired - that's okay. - aCookieAttributes.expiryTime = aCurrentTime + maxage; + aCookie.expiryTime = aCurrentTime + maxage; // check for expires attribute - } else if (!aCookieAttributes.expires.IsEmpty()) { + } else if (!aCookie.expires.IsEmpty()) { PRTime expires; // parse expiry time - if (PR_ParseTimeString(aCookieAttributes.expires.get(), true, &expires) != PR_SUCCESS) { + if (PR_ParseTimeString(aCookie.expires.get(), true, &expires) != PR_SUCCESS) { return true; } @@ -4240,7 +4267,7 @@ nsCookieService::GetExpiry(nsCookieAttributes &aCookieAttributes, // Because if current time be set in the future, but the cookie expire // time be set less than current time and more than server time. // The cookie item have to be used to the expired cookie. - aCookieAttributes.expiryTime = expires / int64_t(PR_USEC_PER_SEC); + aCookie.expiryTime = expires / int64_t(PR_USEC_PER_SEC); // default to session cookie if no attributes found } else { diff --git a/system/network/cookie/nsCookieService.h b/system/network/cookie/nsCookieService.h index 185f0b492..deb9fed33 100644 --- a/system/network/cookie/nsCookieService.h +++ b/system/network/cookie/nsCookieService.h @@ -307,6 +307,7 @@ class nsCookieService final : public nsICookieService CookieStatus CheckPrefs(nsIURI *aHostURI, bool aIsForeign, const char *aCookieHeader); bool CheckDomain(nsCookieAttributes &aCookie, nsIURI *aHostURI, const nsCString &aBaseDomain, bool aRequireHostMatch); static bool CheckPath(nsCookieAttributes &aCookie, nsIURI *aHostURI); + static bool CheckHiddenPrefix(nsCookieAttributes &aCookie); static bool CheckPrefixes(nsCookieAttributes &aCookie, bool aSecureRequest); static bool GetExpiry(nsCookieAttributes &aCookie, int64_t aServerTime, int64_t aCurrentTime); void RemoveAllFromMemory(); -- cgit v1.2.3