[network] Improve checks while parsing MIME parameters.

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
 // <token> [ ';' <token> '=' <token-or-quoted-string> ]*
 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 = [