From bc89e58def2ce07baa8d5a6ce3f24e5ecb95c346 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Wed, 12 Apr 2023 16:41:04 +0200 Subject: [network] Improve checks while parsing MIME parameters. --- netwerk/mime/nsIMIMEHeaderParam.idl | 4 +-- netwerk/mime/nsMIMEHeaderParamImpl.cpp | 64 +++++++++++++++++++++++++--------- netwerk/mime/nsMIMEHeaderParamImpl.h | 13 +++---- netwerk/test/unit/test_MIME_params.js | 10 ++++++ 4 files changed, 67 insertions(+), 24 deletions(-) diff --git a/netwerk/mime/nsIMIMEHeaderParam.idl b/netwerk/mime/nsIMIMEHeaderParam.idl index ffe8403d60..84397529fb 100644 --- a/netwerk/mime/nsIMIMEHeaderParam.idl +++ b/netwerk/mime/nsIMIMEHeaderParam.idl @@ -12,7 +12,7 @@ #include "nsISupports.idl" -[scriptable, uuid(9c9252a1-fdaf-40a2-9c2b-a3dc45e28dde)] +[scriptable, uuid(6e8476d5-ba95-4fee-85d9-f96729b387d8)] interface nsIMIMEHeaderParam : nsISupports { /** @@ -132,7 +132,7 @@ interface nsIMIMEHeaderParam : nsISupports { */ [noscript] - string getParameterInternal(in string aHeaderVal, + string getParameterInternal(in ACString aHeaderVal, in string aParamName, out string aCharset, out string aLang); diff --git a/netwerk/mime/nsMIMEHeaderParamImpl.cpp b/netwerk/mime/nsMIMEHeaderParamImpl.cpp index bd5c25ce4b..68e8c86573 100644 --- a/netwerk/mime/nsMIMEHeaderParamImpl.cpp +++ b/netwerk/mime/nsMIMEHeaderParamImpl.cpp @@ -65,6 +65,28 @@ nsMIMEHeaderParamImpl::GetParameterHTTP(const nsACString& aHeaderVal, aFallbackCharset, aTryLocaleCharset, aLang, aResult); } +/* static */ +// Detects the presence of any non-null characters past null in a header. +bool +nsMIMEHeaderParamImpl::ContainsTrailingCharPastNull(const nsACString& aVal) { + nsACString::const_iterator first; + aVal.BeginReading(first); + nsACString::const_iterator end; + aVal.EndReading(end); + + if (FindCharInReadable(L'\0', first, end)) { + while (first != end) { + if (*first != '\0') { + // Header contains trailing characters past the null character + return true; + } + ++first; + } + } + // No stray non-null characters found. + return false; +} + // XXX : aTryLocaleCharset is not yet effective. nsresult nsMIMEHeaderParamImpl::DoGetParameter(const nsACString& aHeaderVal, @@ -81,9 +103,8 @@ nsMIMEHeaderParamImpl::DoGetParameter(const nsACString& aHeaderVal, // aDecoding (5987 being a subset of 2231) and return charset.) nsXPIDLCString med; nsXPIDLCString charset; - rv = DoParameterInternal(PromiseFlatCString(aHeaderVal).get(), aParamName, - aDecoding, getter_Copies(charset), aLang, - getter_Copies(med)); + rv = DoParameterInternal(aHeaderVal, aParamName, aDecoding, + getter_Copies(charset), aLang, getter_Copies(med)); if (NS_FAILED(rv)) return rv; @@ -346,11 +367,11 @@ bool IsValidOctetSequenceForCharset(nsACString& aCharset, const char *aOctets) // The format of these header lines is // [ ';' '=' ]* NS_IMETHODIMP -nsMIMEHeaderParamImpl::GetParameterInternal(const char *aHeaderValue, - const char *aParamName, - char **aCharset, - char **aLang, - char **aResult) +nsMIMEHeaderParamImpl::GetParameterInternal(const nsACString& aHeaderValue, + const char* aParamName, + char** aCharset, + char** aLang, + char** aResult) { return DoParameterInternal(aHeaderValue, aParamName, MIME_FIELD_ENCODING, aCharset, aLang, aResult); @@ -358,16 +379,29 @@ nsMIMEHeaderParamImpl::GetParameterInternal(const char *aHeaderValue, nsresult -nsMIMEHeaderParamImpl::DoParameterInternal(const char *aHeaderValue, - const char *aParamName, +nsMIMEHeaderParamImpl::DoParameterInternal(const nsACString& aHeaderValue, + const char* aParamName, ParamDecoding aDecoding, - char **aCharset, - char **aLang, - char **aResult) + char** aCharset, + char** aLang, + char** aResult) { - if (!aHeaderValue || !*aHeaderValue || !aResult) + if (aHeaderValue.IsEmpty() || !aResult) { + return NS_ERROR_INVALID_ARG; + } + + if (ContainsTrailingCharPastNull(aHeaderValue)) { + // See Bug 1784348 return NS_ERROR_INVALID_ARG; + } + + const nsCString& flat = PromiseFlatCString(aHeaderValue); + const char* str = flat.get(); + + if (!*str) { + return NS_ERROR_INVALID_ARG; + } *aResult = nullptr; @@ -380,8 +414,6 @@ nsMIMEHeaderParamImpl::DoParameterInternal(const char *aHeaderValue, // them for HTTP header fields later on, see bug 776324 bool acceptContinuations = true; - const char *str = aHeaderValue; - // skip leading white space. for (; *str && nsCRT::IsAsciiSpace(*str); ++str) ; diff --git a/netwerk/mime/nsMIMEHeaderParamImpl.h b/netwerk/mime/nsMIMEHeaderParamImpl.h index 8918ee3275..9d157b2609 100644 --- a/netwerk/mime/nsMIMEHeaderParamImpl.h +++ b/netwerk/mime/nsMIMEHeaderParamImpl.h @@ -29,13 +29,14 @@ private: char **aLang, nsAString& aResult); - nsresult DoParameterInternal(const char *aHeaderValue, - const char *aParamName, + nsresult DoParameterInternal(const nsACString& aHeaderValue, + const char* aParamName, ParamDecoding aDecoding, - char **aCharset, - char **aLang, - char **aResult); - + char** aCharset, + char** aLang, + char** aResult); + + static bool ContainsTrailingCharPastNull(const nsACString& aVal); }; #endif diff --git a/netwerk/test/unit/test_MIME_params.js b/netwerk/test/unit/test_MIME_params.js index 2c46a061c2..79b41b552f 100644 --- a/netwerk/test/unit/test_MIME_params.js +++ b/netwerk/test/unit/test_MIME_params.js @@ -432,6 +432,16 @@ var tests = [ // Bug 783502 - xpcshell test netwerk/test/unit/test_MIME_params.js fails on AddressSanitizer ['attachment; filename="\\b\\a\\', "attachment", "ba\\"], + + // Bug 1784348 + ["attachment; filename=foo.exe\0.pdf", + Cr.NS_ERROR_ILLEGAL_VALUE, + Cr.NS_ERROR_INVALID_ARG], + ["attachment; filename=\0\0foo\0", + Cr.NS_ERROR_ILLEGAL_VALUE, + Cr.NS_ERROR_INVALID_ARG], + ["attachment; filename=foo\0\0\0", "attachment", "foo"], + ["attachment; filename=\0\0\0", "attachment", ""], ]; var rfc5987paramtests = [ -- cgit v1.2.3