From 5bc9f5b9ac2452b961857628c087eac169adafcd Mon Sep 17 00:00:00 2001 From: sunil mayya Date: Mon, 24 Oct 2022 09:17:22 +0000 Subject: [PATCH] Bug 1790311 - update handling of request headers in Fetch/XHR. r=necko-reviewers,valentin Differential Revision: https://phabricator.services.mozilla.com/D157729 --- dom/base/nsContentUtils.cpp | 45 ++++++++++++++++++++++++---- dom/base/nsContentUtils.h | 11 ++++++- dom/fetch/FetchUtil.h | 1 - dom/fetch/InternalHeaders.cpp | 18 ++++++----- dom/fetch/InternalHeaders.h | 5 ++-- dom/xhr/XMLHttpRequestMainThread.cpp | 3 +- 6 files changed, 66 insertions(+), 17 deletions(-) diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index 04ff5ddfdfdc..d1b3efebfe7a 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -7311,15 +7311,25 @@ bool nsContentUtils::IsNodeInEditableRegion(nsINode* aNode) { } // static -bool nsContentUtils::IsForbiddenRequestHeader(const nsACString& aHeader) { +bool nsContentUtils::IsForbiddenRequestHeader(const nsACString& aHeader, + const nsACString& aValue) { if (IsForbiddenSystemRequestHeader(aHeader)) { return true; } - return StringBeginsWith(aHeader, "proxy-"_ns, - nsCaseInsensitiveCStringComparator) || - StringBeginsWith(aHeader, "sec-"_ns, - nsCaseInsensitiveCStringComparator); + if ((nsContentUtils::IsOverrideMethodHeader(aHeader) && + nsContentUtils::ContainsForbiddenMethod(aValue))) { + return true; + } + + if (StringBeginsWith(aHeader, "proxy-"_ns, + nsCaseInsensitiveCStringComparator) || + StringBeginsWith(aHeader, "sec-"_ns, + nsCaseInsensitiveCStringComparator)) { + return true; + } + + return false; } // static @@ -7359,6 +7369,31 @@ bool nsContentUtils::IsForbiddenResponseHeader(const nsACString& aHeader) { aHeader.LowerCaseEqualsASCII("set-cookie2")); } +// static +bool nsContentUtils::IsOverrideMethodHeader(const nsACString& headerName) { + return headerName.EqualsIgnoreCase("x-http-method-override") || + headerName.EqualsIgnoreCase("x-http-method") || + headerName.EqualsIgnoreCase("x-method-override"); +} + +// static +bool nsContentUtils::ContainsForbiddenMethod(const nsACString& headerValue) { + bool hasInsecureMethod = false; + nsCCharSeparatedTokenizer tokenizer(headerValue, ','); + + while (tokenizer.hasMoreTokens()) { + const nsDependentCSubstring& value = tokenizer.nextToken(); + + if (value.EqualsIgnoreCase("connect") || value.EqualsIgnoreCase("trace") || + value.EqualsIgnoreCase("track")) { + hasInsecureMethod = true; + break; + } + } + + return hasInsecureMethod; +} + // static bool nsContentUtils::IsCorsUnsafeRequestHeaderValue( const nsACString& aHeaderValue) { diff --git a/dom/base/nsContentUtils.h b/dom/base/nsContentUtils.h index c320d7528508..4c8d2b3d2709 100644 --- a/dom/base/nsContentUtils.h +++ b/dom/base/nsContentUtils.h @@ -2764,7 +2764,8 @@ class nsContentUtils { * Returns whether a given header is forbidden for an XHR or fetch * request. */ - static bool IsForbiddenRequestHeader(const nsACString& aHeader); + static bool IsForbiddenRequestHeader(const nsACString& aHeader, + const nsACString& aValue); /** * Returns whether a given header is forbidden for a system XHR @@ -2772,6 +2773,14 @@ class nsContentUtils { */ static bool IsForbiddenSystemRequestHeader(const nsACString& aHeader); + /** + * Checks whether the header overrides any http methods + */ + static bool IsOverrideMethodHeader(const nsACString& headerName); + /** + * Checks whether the header value contains any forbidden method + */ + static bool ContainsForbiddenMethod(const nsACString& headerValue); /** * Returns whether a given header has characters that aren't permitted */ diff --git a/dom/fetch/FetchUtil.h b/dom/fetch/FetchUtil.h index c86019ed0b11..c0c9157a43c8 100644 --- a/dom/fetch/FetchUtil.h +++ b/dom/fetch/FetchUtil.h @@ -37,7 +37,6 @@ class FetchUtil final { */ static nsresult GetValidRequestMethod(const nsACString& aMethod, nsCString& outMethod); - /** * Extracts an HTTP header from a substring range. */ diff --git a/dom/fetch/InternalHeaders.cpp b/dom/fetch/InternalHeaders.cpp index b2146c83173a..6212259c9ded 100644 --- a/dom/fetch/InternalHeaders.cpp +++ b/dom/fetch/InternalHeaders.cpp @@ -6,6 +6,7 @@ #include "mozilla/dom/InternalHeaders.h" +#include "FetchUtil.h" #include "mozilla/dom/FetchTypes.h" #include "mozilla/ErrorResult.h" @@ -56,11 +57,11 @@ bool InternalHeaders::IsValidHeaderValue(const nsCString& aLowerName, } // Step 4 - if (mGuard == HeadersGuardEnum::Request && - IsForbiddenRequestHeader(aLowerName)) { - return false; + if (mGuard == HeadersGuardEnum::Request) { + if (IsForbiddenRequestHeader(aLowerName, aNormalizedValue)) { + return false; + } } - // Step 5 if (mGuard == HeadersGuardEnum::Request_no_cors) { nsAutoCString tempValue; @@ -161,7 +162,9 @@ void InternalHeaders::Delete(const nsACString& aName, ErrorResult& aRv) { } // Step 3 - if (IsForbiddenRequestHeader(lowerName)) { + nsAutoCString value; + GetInternal(lowerName, value, aRv); + if (IsForbiddenRequestHeader(lowerName, value)) { return; } @@ -381,9 +384,10 @@ bool InternalHeaders::IsImmutable(ErrorResult& aRv) const { return false; } -bool InternalHeaders::IsForbiddenRequestHeader(const nsCString& aName) const { +bool InternalHeaders::IsForbiddenRequestHeader(const nsCString& aName, + const nsACString& aValue) const { return mGuard == HeadersGuardEnum::Request && - nsContentUtils::IsForbiddenRequestHeader(aName); + nsContentUtils::IsForbiddenRequestHeader(aName, aValue); } bool InternalHeaders::IsForbiddenRequestNoCorsHeader( diff --git a/dom/fetch/InternalHeaders.h b/dom/fetch/InternalHeaders.h index 2fac237733d7..2841844ef6da 100644 --- a/dom/fetch/InternalHeaders.h +++ b/dom/fetch/InternalHeaders.h @@ -131,7 +131,8 @@ class InternalHeaders final { bool IsValidHeaderValue(const nsCString& aLowerName, const nsCString& aNormalizedValue, ErrorResult& aRv); bool IsImmutable(ErrorResult& aRv) const; - bool IsForbiddenRequestHeader(const nsCString& aName) const; + bool IsForbiddenRequestHeader(const nsCString& aName, + const nsACString& aValue) const; bool IsForbiddenRequestNoCorsHeader(const nsCString& aName) const; bool IsForbiddenRequestNoCorsHeader(const nsCString& aName, const nsACString& aValue) const; @@ -144,7 +145,7 @@ class InternalHeaders final { bool IsInvalidMutableHeader(const nsCString& aName, const nsACString& aValue, ErrorResult& aRv) const { return IsInvalidName(aName, aRv) || IsInvalidValue(aValue, aRv) || - IsImmutable(aRv) || IsForbiddenRequestHeader(aName) || + IsImmutable(aRv) || IsForbiddenRequestHeader(aName, aValue) || IsForbiddenRequestNoCorsHeader(aName, aValue) || IsForbiddenResponseHeader(aName); } diff --git a/dom/xhr/XMLHttpRequestMainThread.cpp b/dom/xhr/XMLHttpRequestMainThread.cpp index d0aca535e017..c21601222e10 100644 --- a/dom/xhr/XMLHttpRequestMainThread.cpp +++ b/dom/xhr/XMLHttpRequestMainThread.cpp @@ -3174,7 +3174,8 @@ void XMLHttpRequestMainThread::SetRequestHeader(const nsACString& aName, // Step 5 bool isPrivilegedCaller = IsSystemXHR(); - bool isForbiddenHeader = nsContentUtils::IsForbiddenRequestHeader(aName); + bool isForbiddenHeader = + nsContentUtils::IsForbiddenRequestHeader(aName, aValue); if (!isPrivilegedCaller && isForbiddenHeader) { AutoTArray params; CopyUTF8toUTF16(aName, *params.AppendElement());