From a57cdde4f1882fd325ee21a4abba4552776e9747 Mon Sep 17 00:00:00 2001
From: Fedor <fedor@mail.ru>
Date: Wed, 12 Aug 2020 09:53:06 +0300
Subject: [PATCH] [network/dom] Improve sanitization of download filenames.

---
 dom/base/nsContentUtils.cpp                         | 8 ++++++++
 netwerk/base/nsBaseChannel.cpp                      | 6 ++++++
 netwerk/protocol/http/HttpBaseChannel.cpp           | 6 ++++++
 uriloader/exthandler/nsExternalHelperAppService.cpp | 9 ++++++---
 4 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp
index 61d10e022..3568ced90 100644
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@@ -5123,6 +5123,14 @@ nsContentUtils::TriggerLink(nsIContent *aContent, nsPresContext *aPresContext,
       fileName.SetIsVoid(true); // No actionable download attribute was found.
     }
 
+    // Sanitize fileNames containing control characters by replacing them with
+    // underscores.
+    if (!fileName.IsVoid()) {
+      for (int i = 0; i < 32; i++) {
+        fileName.ReplaceChar(char16_t(i), '_');
+      }
+    }
+
     handler->OnLinkClick(aContent, aLinkURI,
                          fileName.IsVoid() ? aTargetSpec.get() : EmptyString().get(),
                          fileName, nullptr, nullptr, aIsTrusted, aContent->NodePrincipal());
diff --git a/netwerk/base/nsBaseChannel.cpp b/netwerk/base/nsBaseChannel.cpp
index 2575fac04..51caa546e 100644
--- a/netwerk/base/nsBaseChannel.cpp
+++ b/netwerk/base/nsBaseChannel.cpp
@@ -579,6 +579,12 @@ NS_IMETHODIMP
 nsBaseChannel::SetContentDispositionFilename(const nsAString &aContentDispositionFilename)
 {
   mContentDispositionFilename = new nsString(aContentDispositionFilename);
+
+  // For safety reasons ensure the filename doesn't contain null characters and
+  // replace them with underscores. We may later pass the extension to system
+  // MIME APIs that expect null terminated strings.
+  mContentDispositionFilename->ReplaceChar(char16_t(0), '_');
+
   return NS_OK;
 }
 
diff --git a/netwerk/protocol/http/HttpBaseChannel.cpp b/netwerk/protocol/http/HttpBaseChannel.cpp
index a53022f71..bf8e17537 100644
--- a/netwerk/protocol/http/HttpBaseChannel.cpp
+++ b/netwerk/protocol/http/HttpBaseChannel.cpp
@@ -562,6 +562,12 @@ NS_IMETHODIMP
 HttpBaseChannel::SetContentDispositionFilename(const nsAString& aContentDispositionFilename)
 {
   mContentDispositionFilename = new nsString(aContentDispositionFilename);
+
+  // For safety reasons ensure the filename doesn't contain null characters and
+  // replace them with underscores. We may later pass the extension to system
+  // MIME APIs that expect null terminated strings.
+  mContentDispositionFilename->ReplaceChar(char16_t(0), '_');
+
   return NS_OK;
 }
 
diff --git a/uriloader/exthandler/nsExternalHelperAppService.cpp b/uriloader/exthandler/nsExternalHelperAppService.cpp
index 455ac457a..7393c3941 100644
--- a/uriloader/exthandler/nsExternalHelperAppService.cpp
+++ b/uriloader/exthandler/nsExternalHelperAppService.cpp
@@ -1218,9 +1218,12 @@ nsExternalAppHandler::nsExternalAppHandler(nsIMIMEInfo * aMIMEInfo,
     mTempFileExtension = char16_t('.');
   AppendUTF8toUTF16(aTempFileExtension, mTempFileExtension);
 
-  // replace platform specific path separator and illegal characters to avoid any confusion
-  mSuggestedFileName.ReplaceChar(KNOWN_PATH_SEPARATORS FILE_ILLEGAL_CHARACTERS, '_');
-  mTempFileExtension.ReplaceChar(KNOWN_PATH_SEPARATORS FILE_ILLEGAL_CHARACTERS, '_');
+  // Replace platform specific path separator and illegal characters to avoid any confusion
+  mSuggestedFileName.ReplaceChar(KNOWN_PATH_SEPARATORS, '_');
+  mSuggestedFileName.ReplaceChar(FILE_ILLEGAL_CHARACTERS, ' ');
+  mSuggestedFileName.ReplaceChar(char16_t(0), '_');
+  mTempFileExtension.ReplaceChar(KNOWN_PATH_SEPARATORS, '_');
+  mTempFileExtension.ReplaceChar(FILE_ILLEGAL_CHARACTERS, ' ');
 
   // Remove unsafe bidi characters which might have spoofing implications (bug 511521).
   const char16_t unsafeBidiCharacters[] = {