Persist prompt/block download limiter state on renderer-initiated loads.

chrome/browser/download/download_request_limiter.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/download_request_limiter.h"
 
 #include "base/bind.h"
 #include "base/stl_util.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/common/features.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
+#include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/resource_dispatcher_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_delegate.h"
 #include "url/gurl.h"
 
 #if BUILDFLAG(ANDROID_JAVA_UI)
 #include "chrome/browser/download/download_request_infobar_delegate_android.h"
@@ skipping 11 matching lines @@
 DownloadRequestLimiter::TabDownloadState::TabDownloadState(
     DownloadRequestLimiter* host,
     content::WebContents* contents,
     content::WebContents* originating_web_contents)
     : content::WebContentsObserver(contents),
       web_contents_(contents),
       host_(host),
       status_(DownloadRequestLimiter::ALLOW_ONE_DOWNLOAD),
       download_count_(0),
       factory_(this) {
-  registrar_.Add(
-      this, content::NOTIFICATION_NAV_ENTRY_PENDING,
-      content::Source<NavigationController>(&contents->GetController()));
   registrar_.Add(this, chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED,
                  content::Source<content::WebContents>(contents));
-  NavigationEntry* last_entry = originating_web_contents ?
-      originating_web_contents->GetController().GetLastCommittedEntry() :
-      contents->GetController().GetLastCommittedEntry();
+  NavigationEntry* last_entry =
+      originating_web_contents
+          ? originating_web_contents->GetController().GetLastCommittedEntry()
+          : contents->GetController().GetLastCommittedEntry();
   if (last_entry)
     initial_page_host_ = last_entry->GetURL().host();
 }
 
 DownloadRequestLimiter::TabDownloadState::~TabDownloadState() {
   // We should only be destroyed after the callbacks have been notified.
   DCHECK(callbacks_.empty());
 
   // And we should have invalidated the back pointer.
   DCHECK(!factory_.HasWeakPtrs());
 }
 
-void DownloadRequestLimiter::TabDownloadState::DidNavigateMainFrame(
-    const content::LoadCommittedDetails& details,
-    const content::FrameNavigateParams& params) {
-  switch (status_) {
-    case ALLOW_ONE_DOWNLOAD:
-    case PROMPT_BEFORE_DOWNLOAD:
-      // When the user reloads the page without responding to the infobar, they
-      // are expecting DownloadRequestLimiter to behave as if they had just
-      // initially navigated to this page. See http://crbug.com/171372
-      NotifyCallbacks(false);
-      host_->Remove(this, web_contents());
-      // WARNING: We've been deleted.
-      break;
-    case DOWNLOADS_NOT_ALLOWED:
-    case ALLOW_ALL_DOWNLOADS:
-      // Don't drop this information. The user has explicitly said that they
-      // do/don't want downloads from this host. If they accidentally Accepted
-      // or Canceled, they can adjust the limiter state by adjusting the
-      // automatic downloads content settings. Alternatively, they can copy the
-      // URL into a new tab, which will make a new DownloadRequestLimiter.
-      // See also the initial_page_host_ logic in Observe() for
-      // NOTIFICATION_NAV_ENTRY_PENDING.
-      break;
-    default:
-      NOTREACHED();
+void DownloadRequestLimiter::TabDownloadState::DidStartNavigation(
+    content::NavigationHandle* navigation_handle) {
+  if (!navigation_handle->IsInMainFrame())
+    return;
+
+  // If the navigation is renderer-initiated, ensure that a prompting or
+  // blocking limiter state is not reset, so window.location.href or meta
+  // refresh can't be abused to avoid the limiter.

[Charlie Reis, 2016/05/13 00:36:12]

Might mention where renderer-initiated navigations with user gestures reset the
state, in case that question comes up again.

[dominickn, 2016/05/17 01:31:48]

Done.

+  if (navigation_handle->IsRendererInitiated() &&
+      (status_ == PROMPT_BEFORE_DOWNLOAD || status_ == DOWNLOADS_NOT_ALLOWED)) {
+    return;
+  }
+
+  if (status_ == DownloadRequestLimiter::ALLOW_ALL_DOWNLOADS ||
+      status_ == DownloadRequestLimiter::DOWNLOADS_NOT_ALLOWED) {
+    // User has either allowed all downloads or canceled all downloads. Only
+    // reset the download state if the user is navigating to a different host
+    // (or host is empty).
+    if (!initial_page_host_.empty() &&
+        navigation_handle->GetURL().host() == initial_page_host_) {

[Charlie Reis, 2016/05/13 00:36:12]

Host comparisons always catch my eye, since there's a long list of corner cases.
 :)  Is this something that could be gamed if the site navigated to about:blank
(which is considered same-origin) or a data URL?  I suppose that would only be
an attack for renderer-initiated navigations, though.

(Also, I suppose this is just a move from below, so I won't ask you to fix it in
this CL.)

[dominickn, 2016/05/17 01:31:48]

I think the fact that a prompt/block state is persisted for renderer-initiated
navigations should account for sites which navigate to data URLs or about:blank.
asanka@, do you have anything to add here?

[asanka, 2016/05/19 02:18:30]

I believe this is safe. If the navigation was renderer intiated, then the
status_ is ALLOW_ALL_DOWNLOADS, in which case resetting the tab download state
will only hurt the attacker (i.e. they'd be better off not navigating).

Often overlooked navigation targets like data: and blob: URLs (is the latter
even possible?) should yield an empty host() which should cause the above
condition to fail.

+      return;
+    }
+  }
+
+  NotifyCallbacks(false);
+  host_->Remove(this, web_contents());
+}
+
+void DownloadRequestLimiter::TabDownloadState::DidFinishNavigation(
+    content::NavigationHandle* navigation_handle) {
+  if (!navigation_handle->IsInMainFrame())
+    return;
+
+  // When the status is ALLOW_ALL_DOWNLOADS or DOWNLOADS_NOT_ALLOWED, don't drop
+  // this information. The user has explicitly said that they
+  // do/don't want downloads from this host. If they accidentally Accepted
+  // or Canceled, they can adjust the limiter state by adjusting the
+  // automatic downloads content settings. Alternatively, they can copy
+  // the URL into a new tab, which will make a new DownloadRequestLimiter.
+  // See also the initial_page_host_ logic in DidStartNavigation or Observe()
+  // for NOTIFICATION_NAV_ENTRY_PENDING.
+  if (status_ == ALLOW_ONE_DOWNLOAD ||
+      (status_ == PROMPT_BEFORE_DOWNLOAD &&
+       !navigation_handle->IsRendererInitiated())) {
+    // When the user reloads the page without responding to the infobar,
+    // they are expecting DownloadRequestLimiter to behave as if they had
+    // just initially navigated to this page. See http://crbug.com/171372.
+    // However, explicitly leave the limiter in place if the navigation was
+    // renderer-initiated and we are in a prompt state.
+    NotifyCallbacks(false);
+    host_->Remove(this, web_contents());
+    // WARNING: We've been deleted.
   }
 }
 
 void DownloadRequestLimiter::TabDownloadState::DidGetUserInteraction(
     const blink::WebInputEvent::Type type) {
   if (is_showing_prompt() || type == blink::WebInputEvent::GestureScrollBegin) {
     // Don't change state if a prompt is showing or if the user has scrolled.
     return;
   }
 
@@ skipping 31 matching lines @@
   if (is_showing_prompt())
     return;
 
 #if BUILDFLAG(ANDROID_JAVA_UI)
   DownloadRequestInfoBarDelegateAndroid::Create(
       InfoBarService::FromWebContents(web_contents_), factory_.GetWeakPtr());
 #else
   PermissionBubbleManager* bubble_manager =
       PermissionBubbleManager::FromWebContents(web_contents_);
   if (bubble_manager) {
-    bubble_manager->AddRequest(new DownloadPermissionRequest(
-        factory_.GetWeakPtr()));
+    bubble_manager->AddRequest(
+        new DownloadPermissionRequest(factory_.GetWeakPtr()));
   } else {
     Cancel();
   }
 #endif
 }
 
 void DownloadRequestLimiter::TabDownloadState::SetContentSetting(
     ContentSetting setting) {
   if (!web_contents_)
     return;
   HostContentSettingsMap* settings =
-    DownloadRequestLimiter::GetContentSettings(web_contents_);
+      DownloadRequestLimiter::GetContentSettings(web_contents_);
   if (!settings)
     return;
   settings->SetContentSettingDefaultScope(
       web_contents_->GetURL(), GURL(),
       CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS, std::string(), setting);
 }
 
 void DownloadRequestLimiter::TabDownloadState::Cancel() {
   SetContentSetting(CONTENT_SETTING_BLOCK);
   NotifyCallbacks(false);
 }
 
 void DownloadRequestLimiter::TabDownloadState::CancelOnce() {
   NotifyCallbacks(false);
 }
 
 void DownloadRequestLimiter::TabDownloadState::Accept() {
   SetContentSetting(CONTENT_SETTING_ALLOW);
   NotifyCallbacks(true);
 }
 
 DownloadRequestLimiter::TabDownloadState::TabDownloadState()
     : web_contents_(NULL),
       host_(NULL),
       status_(DownloadRequestLimiter::ALLOW_ONE_DOWNLOAD),
       download_count_(0),
-      factory_(this) {
-}
+      factory_(this) {}
 
 bool DownloadRequestLimiter::TabDownloadState::is_showing_prompt() const {
   return factory_.HasWeakPtrs();
 }
 
 void DownloadRequestLimiter::TabDownloadState::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
-  DCHECK(type == chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED ||
-         type == content::NOTIFICATION_NAV_ENTRY_PENDING);
+  DCHECK(type == chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED);
 
   // Content settings have been updated for our web contents, e.g. via the OIB
   // or the settings page. Check to see if the automatic downloads setting is
   // different to our internal state, and update the internal state to match if
   // necessary. If there is no content setting persisted, then retain the
   // current state and do nothing.
   //
   // NotifyCallbacks is not called as this notification should be triggered when
   // a download is not pending.
-  if (type == chrome::NOTIFICATION_WEB_CONTENT_SETTINGS_CHANGED) {
-    content::WebContents* contents =
-        content::Source<content::WebContents>(source).ptr();
-    DCHECK_EQ(contents, web_contents());
+  content::WebContents* contents =
+      content::Source<content::WebContents>(source).ptr();
+  DCHECK_EQ(contents, web_contents());
 
-    // Fetch the content settings map for this web contents, and extract the
-    // automatic downloads permission value.
-    HostContentSettingsMap* content_settings = GetContentSettings(contents);
-    if (content_settings) {
-      ContentSetting setting = content_settings->GetContentSetting(
-          contents->GetURL(), contents->GetURL(),
-          CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS, std::string());
+  // Fetch the content settings map for this web contents, and extract the
+  // automatic downloads permission value.
+  HostContentSettingsMap* content_settings = GetContentSettings(contents);
+  if (content_settings) {
+    ContentSetting setting = content_settings->GetContentSetting(
+        contents->GetURL(), contents->GetURL(),
+        CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS, std::string());
 
-      // Update the internal state to match if necessary.
-      switch (setting) {
-        case CONTENT_SETTING_ALLOW:
-          set_download_status(ALLOW_ALL_DOWNLOADS);
-          break;
-        case CONTENT_SETTING_BLOCK:
-          set_download_status(DOWNLOADS_NOT_ALLOWED);
-          break;
-        case CONTENT_SETTING_ASK:
-        case CONTENT_SETTING_DEFAULT:
-        case CONTENT_SETTING_SESSION_ONLY:
-          set_download_status(PROMPT_BEFORE_DOWNLOAD);
-          break;
-        case CONTENT_SETTING_NUM_SETTINGS:
-        case CONTENT_SETTING_DETECT_IMPORTANT_CONTENT:
-          NOTREACHED();
-          return;
-      }
+    // Update the internal state to match if necessary.
+    switch (setting) {
+      case CONTENT_SETTING_ALLOW:
+        set_download_status(ALLOW_ALL_DOWNLOADS);
+        break;
+      case CONTENT_SETTING_BLOCK:
+        set_download_status(DOWNLOADS_NOT_ALLOWED);
+        break;
+      case CONTENT_SETTING_ASK:
+      case CONTENT_SETTING_DEFAULT:
+      case CONTENT_SETTING_SESSION_ONLY:
+        set_download_status(PROMPT_BEFORE_DOWNLOAD);
+        break;
+      case CONTENT_SETTING_NUM_SETTINGS:
+      case CONTENT_SETTING_DETECT_IMPORTANT_CONTENT:
+        NOTREACHED();
+        return;
     }
-    return;
   }
-
-  // Otherwise, there is a pending navigation entry.
-  content::NavigationController* controller = &web_contents()->GetController();
-  DCHECK_EQ(controller, content::Source<NavigationController>(source).ptr());
-
-  // NOTE: Resetting state on a pending navigate isn't ideal. In particular it
-  // is possible that queued up downloads for the page before the pending
-  // navigation will be delivered to us after we process this request. If this
-  // happens we may let a download through that we shouldn't have. But this is
-  // rather rare, and it is difficult to get 100% right, so we don't deal with
-  // it.
-  NavigationEntry* entry = controller->GetPendingEntry();
-  if (!entry)
-    return;
-
-  // Redirects don't count.
-  if (ui::PageTransitionIsRedirect(entry->GetTransitionType()))
-    return;
-
-  if (status_ == DownloadRequestLimiter::ALLOW_ALL_DOWNLOADS ||
-      status_ == DownloadRequestLimiter::DOWNLOADS_NOT_ALLOWED) {
-    // User has either allowed all downloads or canceled all downloads. Only
-    // reset the download state if the user is navigating to a different host
-    // (or host is empty).
-    if (!initial_page_host_.empty() && !entry->GetURL().host().empty() &&
-        entry->GetURL().host() == initial_page_host_)
-      return;
-  }
-
-  NotifyCallbacks(false);
-  host_->Remove(this, web_contents());
 }
 
 void DownloadRequestLimiter::TabDownloadState::NotifyCallbacks(bool allow) {
-  set_download_status(allow ?
-      DownloadRequestLimiter::ALLOW_ALL_DOWNLOADS :
-      DownloadRequestLimiter::DOWNLOADS_NOT_ALLOWED);
+  set_download_status(allow ? DownloadRequestLimiter::ALLOW_ALL_DOWNLOADS
+                            : DownloadRequestLimiter::DOWNLOADS_NOT_ALLOWED);
   std::vector<DownloadRequestLimiter::Callback> callbacks;
   bool change_status = false;
 
   // Selectively send first few notifications only if number of downloads exceed
   // kMaxDownloadsAtOnce. In that case, we also retain the infobar instance and
   // don't close it. If allow is false, we send all the notifications to cancel
   // all remaining downloads and close the infobar.
   if (!allow || (callbacks_.size() < kMaxDownloadsAtOnce)) {
     // Null the generated weak pointer so we don't get notified again.
     factory_.InvalidateWeakPtrs();
@@ skipping 19 matching lines @@
 
 // DownloadRequestLimiter ------------------------------------------------------
 
 HostContentSettingsMap* DownloadRequestLimiter::content_settings_ = NULL;
 
 void DownloadRequestLimiter::SetContentSettingsForTesting(
     HostContentSettingsMap* content_settings) {
   content_settings_ = content_settings;
 }
 
-DownloadRequestLimiter::DownloadRequestLimiter()
-    : factory_(this) {
-}
+DownloadRequestLimiter::DownloadRequestLimiter() : factory_(this) {}
 
 DownloadRequestLimiter::~DownloadRequestLimiter() {
   // All the tabs should have closed before us, which sends notification and
   // removes from state_map_. As such, there should be no pending callbacks.
   DCHECK(state_map_.empty());
 }
 
 DownloadRequestLimiter::DownloadStatus
 DownloadRequestLimiter::GetDownloadStatus(content::WebContents* web_contents) {
   TabDownloadState* state = GetDownloadState(web_contents, NULL, false);
@@ skipping 38 matching lines @@
     return;
   }
 
   // Note that because |originating_contents| might go away before
   // OnCanDownloadDecided is invoked, we look it up by |render_process_host_id|
   // and |render_view_id|.
   base::Callback<void(bool)> can_download_callback = base::Bind(
       &DownloadRequestLimiter::OnCanDownloadDecided, factory_.GetWeakPtr(),
       web_contents_getter, request_method, callback);
 
-  originating_contents->GetDelegate()->CanDownload(
-      url,
-      request_method,
-      can_download_callback);
+  originating_contents->GetDelegate()->CanDownload(url, request_method,
+                                                   can_download_callback);
 }
 
 void DownloadRequestLimiter::OnCanDownloadDecided(
     const content::ResourceRequestInfo::WebContentsGetter& web_contents_getter,
     const std::string& request_method,
     const Callback& orig_callback,
     bool allow) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   content::WebContents* originating_contents = web_contents_getter.Run();
   if (!originating_contents || !allow) {
     orig_callback.Run(false);
     return;
   }
 
-  CanDownloadImpl(originating_contents,
-                  request_method,
-                  orig_callback);
+  CanDownloadImpl(originating_contents, request_method, orig_callback);
 }
 
 HostContentSettingsMap* DownloadRequestLimiter::GetContentSettings(
     content::WebContents* contents) {
-  return content_settings_ ? content_settings_ :
-      HostContentSettingsMapFactory::GetForProfile(
-          Profile::FromBrowserContext(contents->GetBrowserContext()));
+  return content_settings_
+             ? content_settings_
+             : HostContentSettingsMapFactory::GetForProfile(
+                   Profile::FromBrowserContext(contents->GetBrowserContext()));
 }
 
 void DownloadRequestLimiter::CanDownloadImpl(
     content::WebContents* originating_contents,
     const std::string& request_method,
     const Callback& callback) {
   DCHECK(originating_contents);
 
-  TabDownloadState* state = GetDownloadState(
-      originating_contents, originating_contents, true);
+  TabDownloadState* state =
+      GetDownloadState(originating_contents, originating_contents, true);
   switch (state->download_status()) {
     case ALLOW_ALL_DOWNLOADS:
-      if (state->download_count() && !(state->download_count() %
+      if (state->download_count() &&
+          !(state->download_count() %
             DownloadRequestLimiter::kMaxDownloadsAtOnce))
         state->set_download_status(PROMPT_BEFORE_DOWNLOAD);
       callback.Run(true);
       state->increment_download_count();
       break;
 
     case ALLOW_ONE_DOWNLOAD:
       state->set_download_status(PROMPT_BEFORE_DOWNLOAD);
       callback.Run(true);
       state->increment_download_count();
       break;
 
     case DOWNLOADS_NOT_ALLOWED:
       callback.Run(false);
       break;
 
     case PROMPT_BEFORE_DOWNLOAD: {
-      HostContentSettingsMap* content_settings = GetContentSettings(
-          originating_contents);
+      HostContentSettingsMap* content_settings =
+          GetContentSettings(originating_contents);
       ContentSetting setting = CONTENT_SETTING_ASK;
       if (content_settings)
         setting = content_settings->GetContentSetting(
-            originating_contents->GetURL(),
-            originating_contents->GetURL(),
-            CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS,
-            std::string());
+            originating_contents->GetURL(), originating_contents->GetURL(),
+            CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS, std::string());
       switch (setting) {
         case CONTENT_SETTING_ALLOW: {
           TabSpecificContentSettings* settings =
-              TabSpecificContentSettings::FromWebContents(
-                  originating_contents);
+              TabSpecificContentSettings::FromWebContents(originating_contents);
           if (settings)
             settings->SetDownloadsBlocked(false);
           callback.Run(true);
           state->increment_download_count();
           return;
         }
         case CONTENT_SETTING_BLOCK: {
           TabSpecificContentSettings* settings =
-              TabSpecificContentSettings::FromWebContents(
-                  originating_contents);
+              TabSpecificContentSettings::FromWebContents(originating_contents);
           if (settings)
             settings->SetDownloadsBlocked(true);
           callback.Run(false);
           return;
         }
         case CONTENT_SETTING_DEFAULT:
         case CONTENT_SETTING_ASK:
         case CONTENT_SETTING_SESSION_ONLY:
           state->PromptUserForDownload(callback);
           state->increment_download_count();
@@ skipping 10 matching lines @@
       NOTREACHED();
   }
 }
 
 void DownloadRequestLimiter::Remove(TabDownloadState* state,
                                     content::WebContents* contents) {
   DCHECK(ContainsKey(state_map_, contents));
   state_map_.erase(contents);
   delete state;
 }