[Backport] Security Bug 823864

Enforce user gesture requirement on browser side for WebUI.

WebContentsImpl now tracks the last time it received an input event that
could be considered user interaction. When the browser process receives
a WebUI message that requires a user gesture, it checks the WebContents
hosting the WebUI to make sure that the user recently interacted with
it.

This also cleans up a few incidental bits of code:
- RenderWidgetHost no longer prefilters events before notifying the
  delegate. This exposed some broken event filtering, tracked at
  https://crbug.com/827659.
- Since the delegate method no longer prefilters input events,
  RenderWidgetHostDelegate::OnUserInteraction() is now named
  RenderWidgetHostDelegate::DidReceiveInputEvent().

Bug: 823864

Reviewed-on: https://chromium-review.googlesource.com/1028484
Change-Id: I50cb1d91751b3b84ca62031943d6531c0b2fba9f
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

8 files changed, 83 insertions, 42 deletions

diff --git a/chromium/content/browser/renderer_host/render_widget_host_delegate.h b/chromium/content/browser/renderer_host/render_widget_host_delegate.h
index 7b0d5cabbdc..a5a044726b3 100644
--- a/chromium/content/browser/renderer_host/render_widget_host_delegate.h
+++ b/chromium/content/browser/renderer_host/render_widget_host_delegate.h
@@ -100,12 +100,10 @@ class CONTENT_EXPORT RenderWidgetHostDelegate {
   // the event itself.
   virtual bool HandleWheelEvent(const blink::WebMouseWheelEvent& event);
 
-  // Notification the user has performed a direct interaction (mouse down,
-  // scroll, raw key down, gesture tap, or browser-initiated navigation) while
-  // focus was on the page. Informs the delegate that a user is interacting with
-  // a site.
-  virtual void OnUserInteraction(RenderWidgetHostImpl* render_widget_host,
-                                 const blink::WebInputEvent::Type type) {}
+  // Notification that an input event from the user was dispatched to the
+  // widget.
+  virtual void DidReceiveInputEvent(RenderWidgetHostImpl* render_widget_host,
+                                    const blink::WebInputEvent::Type type) {}
 
   // Callback to give the browser a chance to handle the specified gesture
   // event before sending it to the renderer.
diff --git a/chromium/content/browser/renderer_host/render_widget_host_impl.cc b/chromium/content/browser/renderer_host/render_widget_host_impl.cc
index c1a8af784c3..9d92c248b9d 100644
--- a/chromium/content/browser/renderer_host/render_widget_host_impl.cc
+++ b/chromium/content/browser/renderer_host/render_widget_host_impl.cc
@@ -2297,12 +2297,7 @@ InputEventAckState RenderWidgetHostImpl::FilterInputEvent(
         event.GetType() == WebInputEvent::kTouchStart) {
       delegate_->FocusOwningWebContents(this);
     }
-    if (event.GetType() == WebInputEvent::kMouseDown ||
-        event.GetType() == WebInputEvent::kGestureScrollBegin ||
-        event.GetType() == WebInputEvent::kTouchStart ||
-        event.GetType() == WebInputEvent::kRawKeyDown) {
-      delegate_->OnUserInteraction(this, event.GetType());
-    }
+    delegate_->DidReceiveInputEvent(this, event.GetType());
   }
 
   return view_ ? view_->FilterInputEvent(event)
diff --git a/chromium/content/browser/web_contents/web_contents_impl.cc b/chromium/content/browser/web_contents/web_contents_impl.cc
index 675a0ab4e00..8b7b8c79634 100644
--- a/chromium/content/browser/web_contents/web_contents_impl.cc
+++ b/chromium/content/browser/web_contents/web_contents_impl.cc
@@ -137,6 +137,7 @@
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/accessibility/ax_tree_combiner.h"
 #include "ui/base/layout.h"
+#include "ui/events/base_event_utils.h"
 #include "ui/events/blink/web_input_event_traits.h"
 #include "ui/gl/gl_switches.h"
 
@@ -1519,6 +1520,20 @@ void WebContentsImpl::WasHidden() {
   should_normally_be_visible_ = false;
 }
 
+bool WebContentsImpl::HasRecentInteractiveInputEvent() const {
+  static constexpr base::TimeDelta kMaxInterval =
+      base::TimeDelta::FromSeconds(5);
+  base::TimeDelta delta =
+      ui::EventTimeForNow() - last_interactive_input_event_time_;
+  // Note: the expectation is that the caller is typically expecting an input
+  // event, e.g. validating that a WebUI message that requires a gesture is
+  // actually attached to a gesture. Logging to UMA here should hopefully give
+  // sufficient data if 5 seconds is actually sufficient (or even too high a
+  // threshhold).
+  UMA_HISTOGRAM_TIMES("Tabs.TimeSinceLastInteraction", delta);
+  return delta <= kMaxInterval;
+}
+
 #if defined(OS_ANDROID)
 std::set<RenderWidgetHostImpl*> WebContentsImpl::GetAllRenderWidgetHosts() {
   std::set<RenderWidgetHostImpl*> set;
@@ -3451,9 +3466,8 @@ void WebContentsImpl::SystemDragEnded(RenderWidgetHost* source_rwh) {
     browser_plugin_embedder_->SystemDragEnded();
 }
 
-void WebContentsImpl::UserGestureDone() {
-  OnUserInteraction(GetRenderViewHost()->GetWidget(),
-                    blink::WebInputEvent::kUndefined);
+void WebContentsImpl::NavigatedByUser() {
+  OnUserInteraction(blink::WebInputEvent::kUndefined);
 }
 
 void WebContentsImpl::SetClosedByUserGesture(bool value) {
@@ -5416,21 +5430,25 @@ bool WebContentsImpl::DidAddMessageToConsole(int32_t level,
                                            source_id);
 }
 
-void WebContentsImpl::OnUserInteraction(
+void WebContentsImpl::DidReceiveInputEvent(
     RenderWidgetHostImpl* render_widget_host,
     const blink::WebInputEvent::Type type) {
+  // Ideally, this list would be based more off of
+  // https://whatwg.org/C/interaction.html#triggered-by-user-activation.
+  if (type != blink::WebInputEvent::kMouseDown &&
+      type != blink::WebInputEvent::kGestureScrollBegin &&
+      type != blink::WebInputEvent::kTouchStart &&
+      type != blink::WebInputEvent::kRawKeyDown)
+    return;
+
   // Ignore unless the widget is currently in the frame tree.
   if (!HasMatchingWidgetHost(&frame_tree_, render_widget_host))
     return;
 
-  for (auto& observer : observers_)
-    observer.DidGetUserInteraction(type);
+  if (type != blink::WebInputEvent::kGestureScrollBegin)
+    last_interactive_input_event_time_ = ui::EventTimeForNow();
 
-  ResourceDispatcherHostImpl* rdh = ResourceDispatcherHostImpl::Get();
-  // Exclude scroll events as user gestures for resource load dispatches.
-  // rdh is NULL in unittests.
-  if (rdh && type != blink::WebInputEvent::kMouseWheel)
-    rdh->OnUserGesture();
+  OnUserInteraction(type);
 }
 
 void WebContentsImpl::FocusOwningWebContents(
@@ -5823,6 +5841,20 @@ void WebContentsImpl::OnPreferredSizeChanged(const gfx::Size& old_size) {
     delegate_->UpdatePreferredSize(this, new_size);
 }
 
+void WebContentsImpl::OnUserInteraction(const blink::WebInputEvent::Type type) {
+  for (auto& observer : observers_)
+    observer.DidGetUserInteraction(type);
+
+  // TODO(https://crbug.com/827659): This used to check if type != kMouseWheel.
+  // However, due to the caller already filtering event types, this would never
+  // be called with type == kMouseWheel so checking for that here is pointless.
+  // However, mouse wheel events *also* generate a kGestureScrollBegin event...
+  // which is *not* filtered out. Maybe they should be?
+  ResourceDispatcherHostImpl* rdh = ResourceDispatcherHostImpl::Get();
+  if (rdh)  // null in unittests. =(
+    rdh->OnUserGesture();
+}
+
 std::unique_ptr<WebUIImpl> WebContentsImpl::CreateWebUI(const GURL& url) {
   std::unique_ptr<WebUIImpl> web_ui = std::make_unique<WebUIImpl>(this);
   WebUIController* controller =
diff --git a/chromium/content/browser/web_contents/web_contents_impl.h b/chromium/content/browser/web_contents/web_contents_impl.h
index 3e28c9c6789..b5768e75188 100644
--- a/chromium/content/browser/web_contents/web_contents_impl.h
+++ b/chromium/content/browser/web_contents/web_contents_impl.h
@@ -289,6 +289,13 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
     return manifest_manager_host_.get();
   }
 
+  // TODO(https://crbug.com/826293): This is a simple mitigation to validate
+  // that an action that requires a user gesture actually has one in the
+  // trustworthy browser process, rather than relying on the untrustworthy
+  // renderer. This should be eventually merged into and accounted for in the
+  // user activation work.
+  bool HasRecentInteractiveInputEvent() const;
+
 #if defined(OS_ANDROID)
   std::set<RenderWidgetHostImpl*> GetAllRenderWidgetHosts();
   void SetImportance(ChildProcessImportance importance);
@@ -418,7 +425,7 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
   RendererPreferences* GetMutableRendererPrefs() override;
   void Close() override;
   void SystemDragEnded(RenderWidgetHost* source_rwh) override;
-  void UserGestureDone() override;
+  void NavigatedByUser() override;
   void SetClosedByUserGesture(bool value) override;
   bool GetClosedByUserGesture() const override;
   int GetMinimumZoomPercent() const override;
@@ -598,8 +605,8 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
                               const base::string16& source_id) override;
   RendererPreferences GetRendererPrefs(
       BrowserContext* browser_context) const override;
-  void OnUserInteraction(RenderWidgetHostImpl* render_widget_host,
-                         const blink::WebInputEvent::Type type) override;
+  void DidReceiveInputEvent(RenderWidgetHostImpl* render_widget_host,
+                            const blink::WebInputEvent::Type type) override;
   void OnIgnoredUIEvent() override;
   void Activate() override;
   void UpdatePreferredSize(const gfx::Size& pref_size) override;
@@ -1293,6 +1300,8 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
   // |delegate_|.
   void OnPreferredSizeChanged(const gfx::Size& old_size);
 
+  void OnUserInteraction(const blink::WebInputEvent::Type type);
+
   // Internal helper to create WebUI objects associated with |this|. |url| is
   // used to determine which WebUI should be created (if any).
   std::unique_ptr<WebUIImpl> CreateWebUI(const GURL& url);
@@ -1506,6 +1515,11 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
   // the WebContents creation time.
   base::TimeTicks last_active_time_;
 
+  // The time that this WebContents last received an 'interactive' input event
+  // from the user. Interactive input events are things like mouse clicks and
+  // keyboard input, but not mouse wheel scrolling or mouse moves.
+  base::TimeTicks last_interactive_input_event_time_;
+
   // See description above setter.
   bool closed_by_user_gesture_;
 
diff --git a/chromium/content/browser/webui/web_ui_impl.cc b/chromium/content/browser/webui/web_ui_impl.cc
index eabd4a4a8e6..560d787e526 100644
--- a/chromium/content/browser/webui/web_ui_impl.cc
+++ b/chromium/content/browser/webui/web_ui_impl.cc
@@ -11,6 +11,7 @@
 
 #include "base/debug/dump_without_crashing.h"
 #include "base/json/json_writer.h"
+#include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "content/browser/child_process_security_policy_impl.h"
@@ -79,7 +80,7 @@ base::string16 WebUI::GetJavascriptCall(
   return result;
 }
 
-WebUIImpl::WebUIImpl(WebContents* contents)
+WebUIImpl::WebUIImpl(WebContentsImpl* contents)
     : bindings_(BINDINGS_POLICY_WEB_UI),
       web_contents_(contents),
       web_contents_observer_(new MainFrameNavigationObserver(this, contents)) {
@@ -122,6 +123,13 @@ void WebUIImpl::OnWebUISend(RenderFrameHost* sender,
   if (!sender->IsCurrent())
     return;
 
+  if (base::EndsWith(message, "RequiringGesture",
+                     base::CompareCase::SENSITIVE) &&
+      !web_contents_->HasRecentInteractiveInputEvent()) {
+    LOG(ERROR) << message << " received without recent user interaction";
+    return;
+  }
+
   ProcessWebUIMessage(source_url, message, args);
 }
 
diff --git a/chromium/content/browser/webui/web_ui_impl.h b/chromium/content/browser/webui/web_ui_impl.h
index b13969e4080..bf4e8c9863f 100644
--- a/chromium/content/browser/webui/web_ui_impl.h
+++ b/chromium/content/browser/webui/web_ui_impl.h
@@ -22,11 +22,12 @@ class Message;
 
 namespace content {
 class RenderFrameHost;
+class WebContentsImpl;
 
 class CONTENT_EXPORT WebUIImpl : public WebUI,
                                  public base::SupportsWeakPtr<WebUIImpl> {
  public:
-  WebUIImpl(WebContents* contents);
+  explicit WebUIImpl(WebContentsImpl* contents);
   ~WebUIImpl() override;
 
   // Called when a RenderFrame is created for a WebUI (reload after a renderer
@@ -109,8 +110,8 @@ class CONTENT_EXPORT WebUIImpl : public WebUI,
   // The WebUIMessageHandlers we own.
   std::vector<std::unique_ptr<WebUIMessageHandler>> handlers_;
 
-  // Non-owning pointer to the WebContents this WebUI is associated with.
-  WebContents* web_contents_;
+  // Non-owning pointer to the WebContentsImpl this WebUI is associated with.
+  WebContentsImpl* web_contents_;
 
   // Notifies this WebUI about notifications in the main frame.
   std::unique_ptr<MainFrameNavigationObserver> web_contents_observer_;
diff --git a/chromium/content/public/browser/web_contents.h b/chromium/content/public/browser/web_contents.h
index e9a6b886972..c8fa3423f30 100644
--- a/chromium/content/public/browser/web_contents.h
+++ b/chromium/content/public/browser/web_contents.h
@@ -623,10 +623,11 @@ class WebContents : public PageNavigator,
   // WebContentsDelegate.
   virtual void SystemDragEnded(RenderWidgetHost* source_rwh) = 0;
 
-  // Notification the user has made a gesture while focus was on the
-  // page. This is used to avoid uninitiated user downloads (aka carpet
-  // bombing), see DownloadRequestLimiter for details.
-  virtual void UserGestureDone() = 0;
+  // The user initiated navigation to this page (as opposed to a navigation that
+  // could have been triggered without user interaction). Used to avoid
+  // uninitiated user downloads (aka carpet bombing), see DownloadRequestLimiter
+  // for details.
+  virtual void NavigatedByUser() = 0;
 
   // Indicates if this tab was explicitly closed by the user (control-w, close
   // tab menu item...). This is false for actions that indirectly close the tab,
diff --git a/chromium/content/renderer/web_ui_extension.cc b/chromium/content/renderer/web_ui_extension.cc
index 93ff2804839..e9e1a6d1e5b 100644
--- a/chromium/content/renderer/web_ui_extension.cc
+++ b/chromium/content/renderer/web_ui_extension.cc
@@ -7,7 +7,6 @@
 #include <memory>
 #include <utility>
 
-#include "base/strings/string_util.h"
 #include "base/values.h"
 #include "content/common/frame_messages.h"
 #include "content/public/common/bindings_policy.h"
@@ -98,13 +97,6 @@ void WebUIExtension::Send(gin::Arguments* args) {
     return;
   }
 
-  if (base::EndsWith(message, "RequiringGesture",
-                     base::CompareCase::SENSITIVE) &&
-      !blink::WebUserGestureIndicator::IsProcessingUserGesture(frame)) {
-    NOTREACHED();
-    return;
-  }
-
   // If they've provided an optional message parameter, convert that into a
   // Value to send to the browser process.
   std::unique_ptr<base::ListValue> content;