Neutralize dangerous subresource files during Save Page.

chrome/browser/download/chrome_download_manager_delegate.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/download/chrome_download_manager_delegate.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 24 matching lines @@
 #include "chrome/browser/platform_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/browser/ui/scoped_tabbed_browser_displayer.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/features.h"
 #include "chrome/common/pref_names.h"
+#include "chrome/common/safe_browsing/file_type_policies.h"
+#include "chrome/grit/generated_resources.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_member.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/download_item.h"
 #include "content/public/browser/download_manager.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/page_navigator.h"
 #include "net/base/filename_util.h"
 #include "net/base/mime_util.h"
+#include "ui/base/l10n/l10n_util.h"
 
 #if BUILDFLAG(ANDROID_JAVA_UI)
 #include "chrome/browser/android/download/chrome_download_manager_overwrite_infobar_delegate.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #endif
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/drive/download_handler.h"
 #include "chrome/browser/chromeos/drive/file_system_util.h"
 #endif
@@ skipping 361 matching lines @@
   // Deletes itself.
   new SavePackageFilePicker(
       web_contents,
       suggested_path,
       default_extension,
       can_save_as_complete,
       download_prefs_.get(),
       callback);
 }
 
+void ChromeDownloadManagerDelegate::SanitizeSaveItemFilename(
+    base::FilePath* filename) {
+  safe_browsing::FileTypePolicies* file_type_policies =
+      safe_browsing::FileTypePolicies::GetInstance();
+  if (!file_type_policies)

[Nathan Parker, 2016/06/13 21:15:00]

This shouldn't be necessary.  The Singleton guarantees it.

+    return;
+
+  if (file_type_policies->GetFileDangerLevel(*filename) ==
+      safe_browsing::DownloadFileType::NOT_DANGEROUS)

[Nathan Parker, 2016/06/13 21:15:00]

So we're not actually checking with safe browsing here, we're just renaming any
file type that's potentially dangerous, correct?  That does mean that things
like .zips (NOT_DANGEROUS) won't be adjusted or scanned. I suspect that's ok,
since that'd be a lot of friction to overcome.... Except... if they followed
this with a .search-ms file to help the user find it.  I'm leaning towards
saying this is sufficient.

[asanka, 2016/06/14 21:24:07]

Yeah. I was a bit on the fence about what to do with supported-but-not-dangerous
file types.

These are:

[
  "bin",
  "pdf",
  "rtf",
  "001",
  "7z",
  "ace",
  "arc",
  "arj",
  "b64",
  "balz",
  "bhx",
  "bz",
  "bz2",
  "bzip2",
  "cab",
  "cpio",
  "fat",
  "gz",
  "gzip",
  "hfs",
  "hqx",
  "iso",
  "lha",
  "lpaq1",
  "lpaq5",
  "lpaq8",
  "lzh",
  "lzma",
  "mim",
  "ntfs",
  "paq8f",
  "paq8jd",
  "paq8l",
  "paq8o",
  "pea",
  "quad",
  "r00",
  "r01",
  "r02",
  "r03",
  "r04",
  "r05",
  "r06",
  "r07",
  "r08",
  "r09",
  "r10",
  "r11",
  "r12",
  "r13",
  "r14",
  "r15",
  "r16",
  "r17",
  "r18",
  "r19",
  "r20",
  "r21",
  "r22",
  "r23",
  "r24",
  "r25",
  "r26",
  "r27",
  "r28",
  "r29",
  "rar",
  "squashfs",
  "swm",
  "tar",
  "taz",
  "tbz",
  "tbz2",
  "tgz",
  "tpz",
  "txz",
  "tz",
  "udf",
  "uu",
  "uue",
  "vhd",
  "vhdx",
  "vmdk",
  "wim",
  "wrc",
  "xar",
  "xxe",
  "xz",
  "z",
  "zip",
  "zipx",
  "zpaq",
  "torrent",
  "svg",
  "apk"
]

By the way, why aren't the archive file types considered dangerous? If safe
browsing is enabled, then we won't warn since we wait to act on the SB ping. If
safe browsing is disabled shouldn't Chrome prompt for these file types?

[Nathan Parker, 2016/06/15 00:01:21]

While it's true that an archive can contain an executable, I think it would be
confusing to warn the user if we didn't KNOW it had an executable. I suspect
we'd lose credibility if we said "this .zip file can harm your computer" when
it's just full of text files.  Ya?

[asanka, 2016/06/16 18:35:16]

On 2016/06/15 at 00:01:21, Nathan Parker wrote:
[...]
True. The danger classifications for archives predate Chrome's ability to look
inside them. So at the time the only option was to warn the user.

Also, as far as the issue of archive formats as subresources; there's sufficient
friction here that I don't feel special handling is necessary.

+    return;
+
+  base::FilePath::FilePath default_filename = base::FilePath::FromUTF8Unsafe(
+      l10n_util::GetStringUTF8(IDS_DEFAULT_DOWNLOAD_FILENAME));
+  *filename = filename->AddExtension(default_filename.BaseName().value());
+}
+
 void ChromeDownloadManagerDelegate::OpenDownloadUsingPlatformHandler(
     DownloadItem* download) {
   base::FilePath platform_path(
       GetPlatformDownloadPath(profile_, download, PLATFORM_TARGET_PATH));
   DCHECK(!platform_path.empty());
   platform_util::OpenItem(profile_, platform_path, platform_util::OPEN_FILE,
                           platform_util::OpenOperationCallback());
 }
 
 void ChromeDownloadManagerDelegate::OpenDownload(DownloadItem* download) {
@@ skipping 327 matching lines @@
       path.MatchesExtension(FILE_PATH_LITERAL(".xht")) ||
       path.MatchesExtension(FILE_PATH_LITERAL(".xhtm")) ||
       path.MatchesExtension(FILE_PATH_LITERAL(".xhtml")) ||
       path.MatchesExtension(FILE_PATH_LITERAL(".xsl")) ||
       path.MatchesExtension(FILE_PATH_LITERAL(".xslt"))) {
     return true;
   }
 #endif
   return false;
 }