[Backport] Use better fallback URLs when calling AVScanFile().

Windows Attachment Services defaults to treating downloads of data: URLs
and other unknown URL schemes as being trustworthy. In addition, source URLs
that are longer than INTERNET_MAX_URL_LENGTH could also cause the
mark-of-the-web to not be applied.

This CL attempts to mitigate these cases by changing base_file_win.cc to
use the referrer URL if one is available and is either http or https.
Furthermore, if the source URL is too long or is empty,
quarantine_win.cc will use the safe fallback "about:internet" as the
source URL.

In addition, quarantine_win.cc now also sets the Referrer field if a
valid referrer is found.

BUG=601538

Review-Url: https://codereview.chromium.org/2025103002
Cr-Commit-Position: refs/heads/master@{#421000}
(CVE-2016-5214)

Change-Id: Ie25070994b6ba9072813453aba460a1389ce5442
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>

3 files changed, 65 insertions, 6 deletions

diff --git a/chromium/content/browser/download/base_file_win.cc b/chromium/content/browser/download/base_file_win.cc
index 5fa373a43ec..17625a9727f 100644
--- a/chromium/content/browser/download/base_file_win.cc
+++ b/chromium/content/browser/download/base_file_win.cc
@@ -316,6 +316,35 @@ DownloadInterruptReason MapScanAndSaveErrorCodeToInterruptReason(
   }
 }
 
+// Given a source and a referrer, determines the "safest" URL that can be used
+// to determine the authority of the download source. Returns an empty URL if no
+// HTTP/S URL can be determined for the <|source_url|, |referrer_url|> pair.
+GURL GetEffectiveAuthorityURL(const GURL& source_url,
+                              const GURL& referrer_url) {
+  if (source_url.is_valid()) {
+    // http{,s} has an authority and are supported.
+    if (source_url.SchemeIsHTTPOrHTTPS())
+      return source_url;
+
+    // If the download source is file:// ideally we should copy the MOTW from
+    // the original file, but given that Chrome/Chromium places strict
+    // restrictions on which schemes can reference file:// URLs, this code is
+    // going to assume that at this point it's okay to treat this download as
+    // being from the local system.
+    if (source_url.SchemeIsFile())
+      return source_url;
+
+    // ftp:// has an authority.
+    if (source_url.SchemeIs(url::kFtpScheme))
+      return source_url;
+  }
+
+  if (referrer_url.is_valid() && referrer_url.SchemeIsHTTPOrHTTPS())
+    return referrer_url;
+
+  return GURL();
+}
+
 } // namespace
 
 // Renames a file using the SHFileOperation API to ensure that the target file
@@ -370,7 +399,10 @@ DownloadInterruptReason BaseFile::AnnotateWithSourceInformation(
       guid = GUID_NULL;
   }
 
-  HRESULT hr = AVScanFile(full_path_, source_url.spec(), guid);
+  HRESULT hr = AVScanFile(full_path_,
+                          GetEffectiveAuthorityURL(source_url, referrer_url).spec(),
+                          referrer_url.spec(),
+                          guid);
 
   // If the download file is missing after the call, then treat this as an
   // interrupted download.
diff --git a/chromium/content/browser/safe_util_win.cc b/chromium/content/browser/safe_util_win.cc
index 820aab5b468..c6b65235be5 100644
--- a/chromium/content/browser/safe_util_win.cc
+++ b/chromium/content/browser/safe_util_win.cc
@@ -4,6 +4,7 @@
 
 #include <shlobj.h>
 #include <shobjidl.h>
+#include <wininet.h>
 
 #include "content/browser/safe_util_win.h"
 
@@ -53,6 +54,7 @@ bool SetInternetZoneIdentifierDirectly(const base::FilePath& full_path) {
 
 HRESULT AVScanFile(const base::FilePath& full_path,
                    const std::string& source_url,
+                   const std::string& referrer_url,
                    const GUID& client_guid) {
   base::win::ScopedComPtr<IAttachmentExecute> attachment_services;
   HRESULT hr = attachment_services.CreateInstance(CLSID_AttachmentServices);
@@ -68,6 +70,10 @@ HRESULT AVScanFile(const base::FilePath& full_path,
     return hr;
   }
 
+  // Note that it is mandatory to check the return values from here on out. If
+  // setting one of the parameters fails, it could leave the object in a state
+  // where the final Save() call will also fail.
+
   if (!IsEqualGUID(client_guid, GUID_NULL)) {
     hr = attachment_services->SetClientGuid(client_guid);
     if (FAILED(hr))
@@ -78,13 +84,31 @@ HRESULT AVScanFile(const base::FilePath& full_path,
   if (FAILED(hr))
     return hr;
 
-  // Note: SetSource looks like it needs to be called, even if empty.
-  // Docs say it is optional, but it appears not calling it at all sets
-  // a zone that is too restrictive.
-  hr = attachment_services->SetSource(base::UTF8ToWide(source_url).c_str());
+  // The source URL could be empty if it was not a valid URL or was not HTTP/S.
+  // If so, user "about:internet" as a fallback URL. The latter is known to
+  // reliably map to the Internet zone.
+  //
+  // In addition, URLs that are longer than INTERNET_MAX_URL_LENGTH are also
+  // known to cause problems for URLMon. Hence also use "about:internet" in
+  // these cases. See http://crbug.com/601538.
+  hr = attachment_services->SetSource(
+      source_url.empty() || source_url.size() > INTERNET_MAX_URL_LENGTH
+          ? L"about:internet"
+          : base::UTF8ToWide(source_url).c_str());
   if (FAILED(hr))
     return hr;
 
+  // Only set referrer if one is present and shorter than
+  // INTERNET_MAX_URL_LENGTH. Also, the source_url is authoritative for
+  // determining the relative danger of |full_path| so we don't consider it an
+  // error if we have to skip the |referrer_url|.
+  if (!referrer_url.empty() && referrer_url.size() < INTERNET_MAX_URL_LENGTH) {
+    hr = attachment_services->SetReferrer(
+        base::UTF8ToWide(referrer_url).c_str());
+    if (FAILED(hr))
+      return false;
+  }
+
   // A failure in the Save() call below could result in the downloaded file
   // being deleted.
   return attachment_services->Save();
diff --git a/chromium/content/browser/safe_util_win.h b/chromium/content/browser/safe_util_win.h
index ce044c79a94..5380a2e79b3 100644
--- a/chromium/content/browser/safe_util_win.h
+++ b/chromium/content/browser/safe_util_win.h
@@ -41,12 +41,15 @@ namespace content {
 // |full_path| : is the path to the downloaded file. This should be the final
 //               path of the download. Must be present.
 // |source_url|: the source URL for the download. If empty, the source will
-//               not be set.
+//               be set to 'about:internet'.
+// |referrer_url|: the referrer URL for the download. If empty, the referrer
+//               will not be set.
 // |client_guid|: the GUID to be set in the IAttachmentExecute client slot.
 //                Used to identify the app to the system AV function.
 //                If GUID_NULL is passed, no client GUID is set.
 HRESULT AVScanFile(const base::FilePath& full_path,
                    const std::string& source_url,
+                   const std::string& referrer_url,
                    const GUID& client_guid);
 }  // namespace content