[SafeBrowsing] Block dangerous unchecked downloads based on a Finch trial.

chrome/browser/file_select_helper.cc

Index: chrome/browser/file_select_helper.cc
diff --git a/chrome/browser/file_select_helper.cc b/chrome/browser/file_select_helper.cc
index 034ec98cf8247f908577b714fc3192898eec8e8a..737b62897c34b14929bdc9df278e64db564be422 100644
--- a/chrome/browser/file_select_helper.cc
+++ b/chrome/browser/file_select_helper.cc
@@ -32,6 +32,7 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/file_chooser_file_info.h"
 #include "content/public/common/file_chooser_params.h"
+#include "net/base/filename_util.h"
 #include "net/base/mime_util.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/shell_dialogs/selected_file_info.h"
@@ -41,6 +42,10 @@
 #include "content/public/browser/site_instance.h"
 #endif
 
+#if defined(FULL_SAFE_BROWSING)
+#include "chrome/browser/safe_browsing/unverified_download_policy.h"
+#endif
+
 using content::BrowserThread;
 using content::FileChooserParams;
 using content::RenderViewHost;
@@ -434,6 +439,13 @@ void FileSelectHelper::RunFileChooserOnFileThread(
 
 void FileSelectHelper::RunFileChooserOnUIThread(
     const FileChooserParams& params) {
+  DCHECK_IMPLIES(!params.default_file_name.empty(),

[sky, 2015/10/21 22:46:19]

Please don't use DCHECK_IMPLIES. Yes, it is in base/logging, but it hurts
readability. I hadn't a clue what it means, which likely goes for most of the
team. Worse yet, looking at the definition I have to map it to:

DCHECK_IMPLIES(!!params.default_file_name.empty()

If you had instead used:

DCHECK(params.default_file_name.empty() || ...)

I would immediately understand what you're checking.

[asanka, 2015/10/22 21:55:12]

Done :), and point taken.

+                 params.mode == FileChooserParams::Save)
+      << "The default_file_name parameter should only be specified for Save "
+         "file choosers";
+  DCHECK(params.default_file_name == params.default_file_name.BaseName())
+      << "The default_file_name parameter should not contain path separators";
+
   if (!render_view_host_ || !web_contents_ || !IsValidProfile(profile_)) {
     // If the renderer was destroyed before we started, just cancel the
     // operation.
@@ -441,6 +453,26 @@ void FileSelectHelper::RunFileChooserOnUIThread(
     return;
   }
 
+  // The parameters contain potentially unsafe filenames provided by the

[sky, 2015/10/21 22:46:20]

I think you should move this to a standalone function that is easy to test
without having to go through a browser test.

[asanka, 2015/10/22 21:55:12]

Done and added a test to FileSelectHelpTest unit test to verify the aspects of
sanitization that's relevant to this class. However, I left one browser_test in
place that has an invalid filename because I wanted something to verify that the
sanitization method is called appropriately.

+  // renderer. They should be sanitized before being used in a file selection
+  // dialog. Also, the subsequent file type blocking checks rely on the file
+  // extension being valid and significant (e.g. no trailing periods on OS_WIN
+  // which disappear once the file is written).
+  base::FilePath default_file_name = net::GenerateFileName(
+      GURL(), std::string(), std::string(),
+      params.default_file_name.AsUTF8Unsafe(), std::string(), std::string());
+  base::FilePath default_file_path =
+      profile_->last_selected_directory().Append(default_file_name);
+
+#if defined(FULL_SAFE_BROWSING)
+  if (params.mode == FileChooserParams::Save &&
+      !safe_browsing::IsUnverifiedDownloadAllowed(
+          default_file_path)) {
+    NotifyRenderViewHostAndEnd(std::vector<ui::SelectedFileInfo>());
+    return;
+  }
+#endif
+
   select_file_dialog_ = ui::SelectFileDialog::Create(
       this, new ChromeSelectFilePolicy(web_contents_));
   if (!select_file_dialog_.get())
@@ -466,10 +498,6 @@ void FileSelectHelper::RunFileChooserOnUIThread(
       NOTREACHED();
   }
 
-  base::FilePath default_file_name = params.default_file_name.IsAbsolute() ?
-      params.default_file_name :
-      profile_->last_selected_directory().Append(params.default_file_name);
-
   gfx::NativeWindow owning_window = platform_util::GetTopLevel(
       render_view_host_->GetWidget()->GetView()->GetNativeView());
 
@@ -480,10 +508,7 @@ void FileSelectHelper::RunFileChooserOnUIThread(
 #endif
 
   select_file_dialog_->SelectFile(
-      dialog_type_,
-      params.title,
-      default_file_name,
-      select_file_types_.get(),
+      dialog_type_, params.title, default_file_path, select_file_types_.get(),
       select_file_types_.get() && !select_file_types_->extensions.empty()
           ? 1
           : 0,  // 1-based index of default extension to show.