For downloads requiring a user gesture, also require the user to have visited the site before today, to hamper attackers.

BUG=81741
TEST=.exe downloads on Windows triggered by an explicit link click should prompt you to confirm iff they are hosted on a site you have not visited before today.
Review URL: http://codereview.chromium.org/7065015

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@86518 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 111 insertions, 57 deletions

diff --git a/chrome/browser/download/download_create_info.cc b/chrome/browser/download/download_create_info.cc
index 4a64b64..8a77e82 100644
--- a/chrome/browser/download/download_create_info.cc
+++ b/chrome/browser/download/download_create_info.cc
@@ -28,8 +28,6 @@ DownloadCreateInfo::DownloadCreateInfo(const FilePath& path,
       has_user_gesture(has_user_gesture),
       db_handle(0),
       prompt_user_for_save_location(false),
-      is_dangerous_file(false),
-      is_dangerous_url(false),
       is_extension_install(false) {
 }
 
@@ -42,8 +40,6 @@ DownloadCreateInfo::DownloadCreateInfo()
       has_user_gesture(false),
       db_handle(0),
       prompt_user_for_save_location(false),
-      is_dangerous_file(false),
-      is_dangerous_url(false),
       is_extension_install(false) {
 }
 
diff --git a/chrome/browser/download/download_create_info.h b/chrome/browser/download/download_create_info.h
index 6e097ef..5adeac2 100644
--- a/chrome/browser/download/download_create_info.h
+++ b/chrome/browser/download/download_create_info.h
@@ -95,12 +95,6 @@ struct DownloadCreateInfo {
   // default location.
   bool prompt_user_for_save_location;
 
-  // Whether this download file is potentially dangerous (ex: exe, dll, ...).
-  bool is_dangerous_file;
-
-  // If safebrowsing believes this URL leads to malware.
-  bool is_dangerous_url;
-
   // The original name for a dangerous download.
   FilePath original_name;
 
diff --git a/chrome/browser/download/download_history.cc b/chrome/browser/download/download_history.cc
index b19f240..e546917 100644
--- a/chrome/browser/download/download_history.cc
+++ b/chrome/browser/download/download_history.cc
@@ -39,6 +39,25 @@ void DownloadHistory::Load(HistoryService::DownloadQueryCallback* callback) {
   hs->CleanUpInProgressEntries();
 }
 
+void DownloadHistory::CheckVisitedReferrerBefore(
+    int32 download_id,
+    const GURL& referrer_url,
+    VisitedBeforeDoneCallback* callback) {
+  DCHECK(callback);
+
+  if (referrer_url.is_valid()) {
+    HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
+    if (hs) {
+      HistoryService::Handle handle = hs->GetVisitCountToHost(referrer_url,
+          &history_consumer_,
+          NewCallback(this, &DownloadHistory::OnGotVisitCountToHost));
+      visited_before_requests_[handle] = std::make_pair(download_id, callback);
+      return;
+    }
+  }
+  callback->Run(download_id, false);
+}
+
 void DownloadHistory::AddEntry(
     DownloadItem* download_item,
     HistoryService::DownloadCreateCallback* callback) {
@@ -52,7 +71,6 @@ void DownloadHistory::AddEntry(
   // handles, so we use a negative value. Eventually, they could overlap, but
   // you'd have to do enough downloading that your ISP would likely stab you in
   // the neck first. YMMV.
-  // TODO(paulg) see bug 958058. EXPLICIT_ACCESS below is wrong.
   HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
   if (download_item->is_otr() || download_item->is_extension_install() ||
       download_item->is_temporary() || !hs) {
@@ -74,7 +92,6 @@ void DownloadHistory::UpdateEntry(DownloadItem* download_item) {
   if (download_item->db_handle() <= kUninitializedHandle)
     return;
 
-  // TODO(paulg) see bug 958058. EXPLICIT_ACCESS below is wrong.
   HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
   if (!hs)
     return;
@@ -90,7 +107,6 @@ void DownloadHistory::UpdateDownloadPath(DownloadItem* download_item,
   if (download_item->db_handle() <= kUninitializedHandle)
     return;
 
-  // TODO(paulg) see bug 958058. EXPLICIT_ACCESS below is wrong.
   HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
   if (hs)
     hs->UpdateDownloadPath(new_path, download_item->db_handle());
@@ -101,7 +117,6 @@ void DownloadHistory::RemoveEntry(DownloadItem* download_item) {
   if (download_item->db_handle() <= kUninitializedHandle)
     return;
 
-  // TODO(paulg) see bug 958058. EXPLICIT_ACCESS below is wrong.
   HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
   if (hs)
     hs->RemoveDownload(download_item->db_handle());
@@ -109,7 +124,6 @@ void DownloadHistory::RemoveEntry(DownloadItem* download_item) {
 
 void DownloadHistory::RemoveEntriesBetween(const base::Time remove_begin,
                                            const base::Time remove_end) {
-  // TODO(paulg) see bug 958058. EXPLICIT_ACCESS below is wrong.
   HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
   if (hs)
     hs->RemoveDownloadsBetween(remove_begin, remove_end);
@@ -118,3 +132,17 @@ void DownloadHistory::RemoveEntriesBetween(const base::Time remove_begin,
 int64 DownloadHistory::GetNextFakeDbHandle() {
   return next_fake_db_handle_--;
 }
+
+void DownloadHistory::OnGotVisitCountToHost(HistoryService::Handle handle,
+                                            bool found_visits,
+                                            int count,
+                                            base::Time first_visit) {
+  VisitedBeforeRequestsMap::iterator request =
+      visited_before_requests_.find(handle);
+  DCHECK(request != visited_before_requests_.end());
+  int32 download_id = request->second.first;
+  VisitedBeforeDoneCallback* callback = request->second.second;
+  visited_before_requests_.erase(request);
+  callback->Run(download_id, found_visits && count &&
+      (first_visit.LocalMidnight() < base::Time::Now().LocalMidnight()));
+}
diff --git a/chrome/browser/download/download_history.h b/chrome/browser/download/download_history.h
index cdde668..63ae23f 100644
--- a/chrome/browser/download/download_history.h
+++ b/chrome/browser/download/download_history.h
@@ -6,6 +6,8 @@
 #define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_HISTORY_H_
 #pragma once
 
+#include <map>
+
 #include "base/basictypes.h"
 #include "chrome/browser/history/history.h"
 #include "content/browser/cancelable_request.h"
@@ -20,6 +22,8 @@ class Time;
 // Interacts with the HistoryService on behalf of the download subsystem.
 class DownloadHistory {
  public:
+  typedef Callback2<int32, bool>::Type VisitedBeforeDoneCallback;
+
   // A fake download table ID which represents a download that has started,
   // but is not yet in the table.
   static const int kUninitializedHandle;
@@ -30,6 +34,11 @@ class DownloadHistory {
   // Retrieves DownloadCreateInfos saved in the history.
   void Load(HistoryService::DownloadQueryCallback* callback);
 
+  // Checks whether |referrer_url| has been visited before today.
+  void CheckVisitedReferrerBefore(int32 download_id,
+                                  const GURL& referrer_url,
+                                  VisitedBeforeDoneCallback* callback);
+
   // Adds a new entry for a download to the history database.
   void AddEntry(DownloadItem* download_item,
                 HistoryService::DownloadCreateCallback* callback);
@@ -52,6 +61,15 @@ class DownloadHistory {
   int64 GetNextFakeDbHandle();
 
  private:
+  typedef std::map<HistoryService::Handle,
+                   std::pair<int32, VisitedBeforeDoneCallback*> >
+      VisitedBeforeRequestsMap;
+
+  void OnGotVisitCountToHost(HistoryService::Handle handle,
+                             bool found_visits,
+                             int count,
+                             base::Time first_visit);
+
   Profile* profile_;
 
   // In case we don't have a valid db_handle, we use |fake_db_handle_| instead.
@@ -62,6 +80,9 @@ class DownloadHistory {
 
   CancelableRequestConsumer history_consumer_;
 
+  // The outstanding requests made by CheckVisitedReferrerBefore().
+  VisitedBeforeRequestsMap visited_before_requests_;
+
   DISALLOW_COPY_AND_ASSIGN(DownloadHistory);
 };
 
diff --git a/chrome/browser/download/download_item.cc b/chrome/browser/download/download_item.cc
index d6602f6..920d767 100644
--- a/chrome/browser/download/download_item.cc
+++ b/chrome/browser/download/download_item.cc
@@ -149,8 +149,8 @@ DownloadItem::DownloadItem(DownloadManager* download_manager,
                            bool is_otr)
     : state_info_(info.original_name, info.save_info.file_path,
                   info.has_user_gesture, info.prompt_user_for_save_location,
-                  info.path_uniquifier, info.is_dangerous_file,
-                  info.is_dangerous_url, info.is_extension_install),
+                  info.path_uniquifier, false, false,
+                  info.is_extension_install),
       process_handle_(info.process_handle),
       download_id_(info.download_id),
       full_path_(info.path),
@@ -170,8 +170,7 @@ DownloadItem::DownloadItem(DownloadManager* download_manager,
       download_manager_(download_manager),
       is_paused_(false),
       open_when_complete_(false),
-      safety_state_(GetSafetyState(info.is_dangerous_file,
-                                   info.is_dangerous_url)),
+      safety_state_(SAFE),
       auto_opened_(false),
       is_otr_(is_otr),
       is_temporary_(!info.save_info.file_path.empty()),
@@ -541,8 +540,16 @@ bool DownloadItem::IsDangerous() const {
   return GetDangerType() != DownloadItem::NOT_DANGEROUS;
 }
 
+void DownloadItem::MarkFileDangerous() {
+  state_info_.is_dangerous_file = true;
+  safety_state_ = GetSafetyState(state_info_.is_dangerous_file,
+                                 state_info_.is_dangerous_url);
+}
+
 void DownloadItem::MarkUrlDangerous() {
   state_info_.is_dangerous_url = true;
+  safety_state_ = GetSafetyState(state_info_.is_dangerous_file,
+                                 state_info_.is_dangerous_url);
 }
 
 DownloadHistoryInfo DownloadItem::GetHistoryInfo() const {
diff --git a/chrome/browser/download/download_item.h b/chrome/browser/download/download_item.h
index 59b23fc..193d605 100644
--- a/chrome/browser/download/download_item.h
+++ b/chrome/browser/download/download_item.h
@@ -277,6 +277,7 @@ class DownloadItem {
   // Why |safety_state_| is not SAFE.
   DangerType GetDangerType() const;
   bool IsDangerous() const;
+  void MarkFileDangerous();
   void MarkUrlDangerous();
 
   bool auto_opened() { return auto_opened_; }
diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc
index 899fe65..58090f2 100644
--- a/chrome/browser/download/download_manager.cc
+++ b/chrome/browser/download/download_manager.cc
@@ -280,10 +280,23 @@ void DownloadManager::CheckDownloadUrlDone(int32 download_id,
   if (is_dangerous_url)
     download->MarkUrlDangerous();
 
-  DownloadStateInfo state = download->state_info();
+  download_history_->CheckVisitedReferrerBefore(download_id,
+      download->referrer_url(),
+      NewCallback(this, &DownloadManager::CheckVisitedReferrerBeforeDone));
+}
+
+void DownloadManager::CheckVisitedReferrerBeforeDone(
+    int32 download_id,
+    bool visited_referrer_before) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  DownloadItem* download = GetActiveDownloadItem(download_id);
+  if (!download)
+    return;
 
   // Check whether this download is for an extension install or not.
   // Allow extensions to be explicitly saved.
+  DownloadStateInfo state = download->state_info();
   if (!state.prompt_user_for_save_location) {
     if (UserScript::IsURLUserScript(download->GetURL(),
         download->mime_type()) ||
@@ -331,8 +344,10 @@ void DownloadManager::CheckDownloadUrlDone(int32 download_id,
     state.suggested_path = state.force_file_name;
   }
 
-  if (!state.prompt_user_for_save_location && state.force_file_name.empty())
-    state.is_dangerous_file = IsDangerous(*download, state);
+  if (!state.prompt_user_for_save_location && state.force_file_name.empty()) {
+    state.is_dangerous_file =
+        IsDangerous(*download, state, visited_referrer_before);
+  }
 
   // We need to move over to the download thread because we don't want to stat
   // the suggested path on the UI thread.
@@ -343,7 +358,7 @@ void DownloadManager::CheckDownloadUrlDone(int32 download_id,
       NewRunnableMethod(
           this,
           &DownloadManager::CheckIfSuggestedPathExists,
-          download_id,
+          download->id(),
           state,
           download_prefs()->download_path()));
 }
@@ -437,8 +452,8 @@ void DownloadManager::CheckIfSuggestedPathExists(int32 download_id,
                         state));
 }
 
-void DownloadManager::OnPathExistenceAvailable(
-    int32 download_id, DownloadStateInfo new_state) {
+void DownloadManager::OnPathExistenceAvailable(int32 download_id,
+                                               DownloadStateInfo new_state) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   DownloadItem* download = GetActiveDownloadItem(download_id);
@@ -1037,7 +1052,8 @@ void DownloadManager::FileSelectionCanceled(void* params) {
 
 // TODO(phajdan.jr): This is apparently not being exercised in tests.
 bool DownloadManager::IsDangerous(const DownloadItem& download,
-                                  const DownloadStateInfo& state) {
+                                  const DownloadStateInfo& state,
+                                  bool visited_referrer_before) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   bool auto_open = ShouldOpenFileBasedOnExtension(state.suggested_path);
@@ -1048,7 +1064,7 @@ bool DownloadManager::IsDangerous(const DownloadItem& download,
     return !(auto_open && state.has_user_gesture);
 
   if (danger_level == download_util::AllowOnUserGesture &&
-      !state.has_user_gesture)
+      (!state.has_user_gesture || !visited_referrer_before))
     return true;
 
   if (state.is_extension_install) {
diff --git a/chrome/browser/download/download_manager.h b/chrome/browser/download/download_manager.h
index f01cb62..fc41675 100644
--- a/chrome/browser/download/download_manager.h
+++ b/chrome/browser/download/download_manager.h
@@ -204,8 +204,6 @@ class DownloadManager
 
   Profile* profile() { return profile_; }
 
-  DownloadHistory* download_history() { return download_history_.get(); }
-
   DownloadPrefs* download_prefs() { return download_prefs_.get(); }
 
   // Creates the download item.  Must be called on the UI thread.
@@ -232,7 +230,8 @@ class DownloadManager
   // user action initiated the download, and whether the user has explicitly
   // marked the file type as "auto open".
   bool IsDangerous(const DownloadItem& download,
-                   const DownloadStateInfo& state);
+                   const DownloadStateInfo& state,
+                   bool visited_referrer_before);
 
   // Called when the user has validated the download of a dangerous file.
   void DangerousDownloadValidated(DownloadItem* download);
@@ -240,6 +239,11 @@ class DownloadManager
   // Callback function after url is checked with safebrowsing service.
   void CheckDownloadUrlDone(int32 download_id, bool is_dangerous_url);
 
+  // Callback function after we check whether the referrer URL has been visited
+  // before today.
+  void CheckVisitedReferrerBeforeDone(int32 download_id,
+                                      bool visited_referrer_before);
+
   // Callback function after download file hash is checked with safebrowsing
   // service.
   void CheckDownloadHashDone(int32 download_id, bool is_dangerous_hash);
diff --git a/chrome/browser/download/download_manager_unittest.cc b/chrome/browser/download/download_manager_unittest.cc
index 9b73e1a..0e947d1 100644
--- a/chrome/browser/download/download_manager_unittest.cc
+++ b/chrome/browser/download/download_manager_unittest.cc
@@ -172,29 +172,21 @@ const struct {
   // Safe download, download finishes BEFORE file name determined.
   // Renamed twice (linear path through UI).  Crdownload file does not need
   // to be deleted.
-  { FILE_PATH_LITERAL("foo.zip"),
-    false, false, true, 2, },
+  { FILE_PATH_LITERAL("foo.zip"), false, false, true, 2, },
   // Dangerous download (file is dangerous or download URL is not safe or both),
   // download finishes BEFORE file name determined. Needs to be renamed only
   // once.
-  { FILE_PATH_LITERAL("Unconfirmed xxx.crdownload"),
-    true, false, true, 1, },
-  { FILE_PATH_LITERAL("Unconfirmed xxx.crdownload"),
-    false, true, true, 1, },
-  { FILE_PATH_LITERAL("Unconfirmed xxx.crdownload"),
-    true, true, true, 1, },
+  { FILE_PATH_LITERAL("Unconfirmed xxx.crdownload"), true, false, true, 1, },
+  { FILE_PATH_LITERAL("Unconfirmed xxx.crdownload"), false, true, true, 1, },
+  { FILE_PATH_LITERAL("Unconfirmed xxx.crdownload"), true, true, true, 1, },
   // Safe download, download finishes AFTER file name determined.
   // Needs to be renamed twice.
-  { FILE_PATH_LITERAL("foo.zip"),
-    false, false, false, 2, },
+  { FILE_PATH_LITERAL("foo.zip"), false, false, false, 2, },
   // Dangerous download, download finishes AFTER file name determined.
   // Needs to be renamed only once.
-  { FILE_PATH_LITERAL("Unconfirmed xxx.crdownload"),
-    true, false, false, 1, },
-  { FILE_PATH_LITERAL("Unconfirmed xxx.crdownload"),
-    false, true, false, 1, },
-  { FILE_PATH_LITERAL("Unconfirmed xxx.crdownload"),
-    true, true, false, 1, },
+  { FILE_PATH_LITERAL("Unconfirmed xxx.crdownload"), true, false, false, 1, },
+  { FILE_PATH_LITERAL("Unconfirmed xxx.crdownload"), false, true, false, 1, },
+  { FILE_PATH_LITERAL("Unconfirmed xxx.crdownload"), true, true, false, 1, },
 };
 
 class MockDownloadFile : public DownloadFile {
@@ -341,8 +333,6 @@ TEST_F(DownloadManagerTest, DownloadRenameTest) {
     info->download_id = static_cast<int>(i);
     info->prompt_user_for_save_location = false;
     info->url_chain.push_back(GURL());
-    info->is_dangerous_file = kDownloadRenameCases[i].is_dangerous_file;
-    info->is_dangerous_url = kDownloadRenameCases[i].is_dangerous_url;
     const FilePath new_path(kDownloadRenameCases[i].suggested_path);
 
     MockDownloadFile* download_file(
@@ -367,6 +357,12 @@ TEST_F(DownloadManagerTest, DownloadRenameTest) {
               2, new_path))));
     }
     download_manager_->CreateDownloadItem(info.get());
+    DownloadItem* download = GetActiveDownloadItem(i);
+    ASSERT_TRUE(download != NULL);
+    if (kDownloadRenameCases[i].is_dangerous_file)
+      download->MarkFileDangerous();
+    if (kDownloadRenameCases[i].is_dangerous_url)
+      download->MarkUrlDangerous();
 
     int32* id_ptr = new int32;
     *id_ptr = i;  // Deleted in FileSelected().
@@ -400,8 +396,6 @@ TEST_F(DownloadManagerTest, DownloadInterruptTest) {
   info->download_id = static_cast<int>(0);
   info->prompt_user_for_save_location = false;
   info->url_chain.push_back(GURL());
-  info->is_dangerous_file = false;
-  info->is_dangerous_url = false;
   const FilePath new_path(FILE_PATH_LITERAL("foo.zip"));
   const FilePath cr_path(download_util::GetCrDownloadPath(new_path));
 
@@ -467,8 +461,6 @@ TEST_F(DownloadManagerTest, DownloadCancelTest) {
   info->download_id = static_cast<int>(0);
   info->prompt_user_for_save_location = false;
   info->url_chain.push_back(GURL());
-  info->is_dangerous_file = false;
-  info->is_dangerous_url = false;
   const FilePath new_path(FILE_PATH_LITERAL("foo.zip"));
   const FilePath cr_path(download_util::GetCrDownloadPath(new_path));
 
@@ -547,8 +539,6 @@ TEST_F(DownloadManagerTest, DownloadOverwriteTest) {
   info->download_id = static_cast<int>(0);
   info->prompt_user_for_save_location = true;
   info->url_chain.push_back(GURL());
-  info->is_dangerous_file = false;
-  info->is_dangerous_url = false;
 
   download_manager_->CreateDownloadItem(info.get());
 
diff --git a/chrome/browser/download/download_state_info.cc b/chrome/browser/download/download_state_info.cc
index 7df6b4f..1838a96 100644
--- a/chrome/browser/download/download_state_info.cc
+++ b/chrome/browser/download/download_state_info.cc
@@ -1,4 +1,3 @@
-
 // Copyright (c) 2011 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.
diff --git a/chrome/browser/renderer_host/download_resource_handler.cc b/chrome/browser/renderer_host/download_resource_handler.cc
index df96cff..b333bf4 100644
--- a/chrome/browser/renderer_host/download_resource_handler.cc
+++ b/chrome/browser/renderer_host/download_resource_handler.cc
@@ -105,8 +105,6 @@ bool DownloadResourceHandler::OnResponseStarted(int request_id,
 
   info->prompt_user_for_save_location =
       save_as_ && save_info_.file_path.empty();
-  info->is_dangerous_file = false;
-  info->is_dangerous_url = false;
   info->referrer_charset = request_->context()->referrer_charset();
   info->save_info = save_info_;
   BrowserThread::PostTask(