From 9103c3e947260f8780fe1a2fb9103bbff7b46fff Mon Sep 17 00:00:00 2001 From: Ben Kelly Date: Thu, 24 Jul 2014 12:38:55 -0400 Subject: [PATCH] Bug 1029620 P4 Make HTTP token and header value validation accessible via nsNetUtil.h r=ehsan r=mcmanus --- content/base/src/nsCrossSiteListenerProxy.cpp | 44 +------------------ content/base/src/nsCrossSiteListenerProxy.h | 3 -- content/base/src/nsXMLHttpRequest.cpp | 4 +- netwerk/base/public/nsNetUtil.h | 14 ++++++ netwerk/base/src/moz.build | 1 + netwerk/base/src/nsNetUtil.cpp | 18 ++++++++ netwerk/protocol/http/HttpBaseChannel.cpp | 16 +++---- netwerk/protocol/http/nsHttp.cpp | 18 ++++++++ netwerk/protocol/http/nsHttp.h | 10 +++-- 9 files changed, 67 insertions(+), 61 deletions(-) create mode 100644 netwerk/base/src/nsNetUtil.cpp diff --git a/content/base/src/nsCrossSiteListenerProxy.cpp b/content/base/src/nsCrossSiteListenerProxy.cpp index 8ef1d785b48..f0d7f261475 100644 --- a/content/base/src/nsCrossSiteListenerProxy.cpp +++ b/content/base/src/nsCrossSiteListenerProxy.cpp @@ -504,46 +504,6 @@ nsCORSListenerProxy::OnStartRequest(nsIRequest* aRequest, return mOuterListener->OnStartRequest(aRequest, aContext); } -bool -IsValidHTTPToken(const nsCSubstring& aToken) -{ - if (aToken.IsEmpty()) { - return false; - } - - nsCSubstring::const_char_iterator iter, end; - - aToken.BeginReading(iter); - aToken.EndReading(end); - - while (iter != end) { - if (*iter <= 32 || - *iter >= 127 || - *iter == '(' || - *iter == ')' || - *iter == '<' || - *iter == '>' || - *iter == '@' || - *iter == ',' || - *iter == ';' || - *iter == ':' || - *iter == '\\' || - *iter == '\"' || - *iter == '/' || - *iter == '[' || - *iter == ']' || - *iter == '?' || - *iter == '=' || - *iter == '{' || - *iter == '}') { - return false; - } - ++iter; - } - - return true; -} - nsresult nsCORSListenerProxy::CheckRequestApproved(nsIRequest* aRequest) { @@ -616,7 +576,7 @@ nsCORSListenerProxy::CheckRequestApproved(nsIRequest* aRequest) if (method.IsEmpty()) { continue; } - if (!IsValidHTTPToken(method)) { + if (!NS_IsValidHTTPToken(method)) { return NS_ERROR_DOM_BAD_URI; } foundMethod |= mPreflightMethod.Equals(method); @@ -635,7 +595,7 @@ nsCORSListenerProxy::CheckRequestApproved(nsIRequest* aRequest) if (header.IsEmpty()) { continue; } - if (!IsValidHTTPToken(header)) { + if (!NS_IsValidHTTPToken(header)) { return NS_ERROR_DOM_BAD_URI; } headers.AppendElement(header); diff --git a/content/base/src/nsCrossSiteListenerProxy.h b/content/base/src/nsCrossSiteListenerProxy.h index 6a54b435a56..4c38daa9d7a 100644 --- a/content/base/src/nsCrossSiteListenerProxy.h +++ b/content/base/src/nsCrossSiteListenerProxy.h @@ -21,9 +21,6 @@ class nsIURI; class nsIParser; class nsIPrincipal; -extern bool -IsValidHTTPToken(const nsCSubstring& aToken); - nsresult NS_StartCORSPreflight(nsIChannel* aRequestChannel, nsIStreamListener* aListener, diff --git a/content/base/src/nsXMLHttpRequest.cpp b/content/base/src/nsXMLHttpRequest.cpp index a2263ec01c0..2d9a403a6eb 100644 --- a/content/base/src/nsXMLHttpRequest.cpp +++ b/content/base/src/nsXMLHttpRequest.cpp @@ -1295,7 +1295,7 @@ nsXMLHttpRequest::IsSafeHeader(const nsACString& header, nsIHttpChannel* httpCha if (token.IsEmpty()) { continue; } - if (!IsValidHTTPToken(token)) { + if (!NS_IsValidHTTPToken(token)) { return false; } if (header.Equals(token, nsCaseInsensitiveCStringComparator())) { @@ -3093,7 +3093,7 @@ nsXMLHttpRequest::SetRequestHeader(const nsACString& header, // Step 3 // Make sure we don't store an invalid header name in mCORSUnsafeHeaders - if (!IsValidHTTPToken(header)) { // XXX nsHttp::IsValidToken? + if (!NS_IsValidHTTPToken(header)) { return NS_ERROR_DOM_SYNTAX_ERR; } diff --git a/netwerk/base/public/nsNetUtil.h b/netwerk/base/public/nsNetUtil.h index 10cf2af82f9..8a517383f02 100644 --- a/netwerk/base/public/nsNetUtil.h +++ b/netwerk/base/public/nsNetUtil.h @@ -2421,4 +2421,18 @@ NS_Get32BitsOfPseudoRandom() #endif } +/** + * Return true if the given string is a reasonable HTTP header value given the + * definition in RFC 2616 section 4.2. Currently we don't pay the cost to do + * full, sctrict validation here since it would require fulling parsing the + * value. + */ +bool NS_IsReasonableHTTPHeaderValue(const nsACString& aValue); + +/** + * Return true if the given string is a valid HTTP token per RFC 2616 section + * 2.2. + */ +bool NS_IsValidHTTPToken(const nsACString& aToken); + #endif // !nsNetUtil_h__ diff --git a/netwerk/base/src/moz.build b/netwerk/base/src/moz.build index 98b709081d1..fb7035808e1 100644 --- a/netwerk/base/src/moz.build +++ b/netwerk/base/src/moz.build @@ -47,6 +47,7 @@ UNIFIED_SOURCES += [ 'nsMIMEInputStream.cpp', 'nsNetAddr.cpp', 'nsNetStrings.cpp', + 'nsNetUtil.cpp', 'nsNetworkZonePolicy.cpp', 'nsPACMan.cpp', 'nsPreloadedStream.cpp', diff --git a/netwerk/base/src/nsNetUtil.cpp b/netwerk/base/src/nsNetUtil.cpp new file mode 100644 index 00000000000..ee98c8dd15b --- /dev/null +++ b/netwerk/base/src/nsNetUtil.cpp @@ -0,0 +1,18 @@ +/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* vim:set ts=4 sw=4 sts=4 et cin: */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "nsNetUtil.h" +#include "nsHttp.h" + +bool NS_IsReasonableHTTPHeaderValue(const nsACString& aValue) +{ + return mozilla::net::nsHttp::IsReasonableHeaderValue(aValue); +} + +bool NS_IsValidHTTPToken(const nsACString& aToken) +{ + return mozilla::net::nsHttp::IsValidToken(aToken); +} diff --git a/netwerk/protocol/http/HttpBaseChannel.cpp b/netwerk/protocol/http/HttpBaseChannel.cpp index 04fa5d7b3dc..b9c24e39b7e 100644 --- a/netwerk/protocol/http/HttpBaseChannel.cpp +++ b/netwerk/protocol/http/HttpBaseChannel.cpp @@ -1119,18 +1119,12 @@ HttpBaseChannel::SetRequestHeader(const nsACString& aHeader, LOG(("HttpBaseChannel::SetRequestHeader [this=%p header=\"%s\" value=\"%s\" merge=%u]\n", this, flatHeader.get(), flatValue.get(), aMerge)); - // Header names are restricted to valid HTTP tokens. - if (!nsHttp::IsValidToken(flatHeader)) - return NS_ERROR_INVALID_ARG; - - // Header values MUST NOT contain line-breaks. RFC 2616 technically - // permits CTL characters, including CR and LF, in header values provided - // they are quoted. However, this can lead to problems if servers do not - // interpret quoted strings properly. Disallowing CR and LF here seems - // reasonable and keeps things simple. We also disallow a null byte. - if (flatValue.FindCharInSet("\r\n") != kNotFound || - flatValue.Length() != strlen(flatValue.get())) + // Verify header names are valid HTTP tokens and header values are reasonably + // close to whats allowed in RFC 2616. + if (!nsHttp::IsValidToken(flatHeader) || + !nsHttp::IsReasonableHeaderValue(flatValue)) { return NS_ERROR_INVALID_ARG; + } nsHttpAtom atom = nsHttp::ResolveAtom(flatHeader.get()); if (!atom) { diff --git a/netwerk/protocol/http/nsHttp.cpp b/netwerk/protocol/http/nsHttp.cpp index 593f562e6a0..79c5b4015f0 100644 --- a/netwerk/protocol/http/nsHttp.cpp +++ b/netwerk/protocol/http/nsHttp.cpp @@ -243,6 +243,24 @@ nsHttp::IsValidToken(const char *start, const char *end) return true; } +// static +bool +nsHttp::IsReasonableHeaderValue(const nsACString &s) +{ + // Header values MUST NOT contain line-breaks. RFC 2616 technically + // permits CTL characters, including CR and LF, in header values provided + // they are quoted. However, this can lead to problems if servers do not + // interpret quoted strings properly. Disallowing CR and LF here seems + // reasonable and keeps things simple. We also disallow a null byte. + const nsACString::char_type* end = s.EndReading(); + for (const nsACString::char_type* i = s.BeginReading(); i != end; ++i) { + if (*i == '\r' || *i == '\n' || *i == '\0') { + return false; + } + } + return true; +} + const char * nsHttp::FindToken(const char *input, const char *token, const char *seps) { diff --git a/netwerk/protocol/http/nsHttp.h b/netwerk/protocol/http/nsHttp.h index e0ecf21ecff..762dd8dbbe0 100644 --- a/netwerk/protocol/http/nsHttp.h +++ b/netwerk/protocol/http/nsHttp.h @@ -132,11 +132,15 @@ struct nsHttp // section 2.2 static bool IsValidToken(const char *start, const char *end); - static inline bool IsValidToken(const nsCString &s) { - const char *start = s.get(); - return IsValidToken(start, start + s.Length()); + static inline bool IsValidToken(const nsACString &s) { + return IsValidToken(s.BeginReading(), s.EndReading()); } + // Returns true if the specified value is reasonable given the defintion + // in RFC 2616 section 4.2. Full strict validation is not performed + // currently as it would require full parsing of the value. + static bool IsReasonableHeaderValue(const nsACString &s); + // find the first instance (case-insensitive comparison) of the given // |token| in the |input| string. the |token| is bounded by elements of // |separators| and may appear at the beginning or end of the |input|