Block renderer-initiated main frame navigations to data URLs

content/browser/frame_host/data_url_navigation_throttle.cc

Index: content/browser/frame_host/data_url_navigation_throttle.cc
diff --git a/content/browser/frame_host/data_url_navigation_throttle.cc b/content/browser/frame_host/data_url_navigation_throttle.cc
new file mode 100644
index 0000000000000000000000000000000000000000..6dde0f39b2833478c42dcc39cc86273892ddd3a9
--- /dev/null
+++ b/content/browser/frame_host/data_url_navigation_throttle.cc
@@ -0,0 +1,49 @@
+// Copyright 2017 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 "content/browser/frame_host/data_url_navigation_throttle.h"
+
+#include "base/memory/ptr_util.h"
+#include "base/strings/stringprintf.h"
+#include "content/public/browser/navigation_handle.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/common/console_message_level.h"
+#include "url/url_constants.h"
+
+namespace content {
+
+namespace {
+const char kConsoleError[] =
+    "Not allowed to top-level navigate to resource: %s";
+}
+
+DataUrlNavigationThrottle::DataUrlNavigationThrottle(
+    NavigationHandle* navigation_handle)
+    : NavigationThrottle(navigation_handle) {}
+
+DataUrlNavigationThrottle::~DataUrlNavigationThrottle() {}
+
+NavigationThrottle::ThrottleCheckResult
+DataUrlNavigationThrottle::WillProcessResponse() {
+  if (navigation_handle()->GetURL().SchemeIs(url::kDataScheme) &&
+      navigation_handle()->IsInMainFrame() &&
+      navigation_handle()->IsRendererInitiated() &&
+      !navigation_handle()->IsDownload()) {
+    navigation_handle()->GetRenderFrameHost()->AddMessageToConsole(
+        CONSOLE_MESSAGE_LEVEL_ERROR,
+        base::StringPrintf(kConsoleError,
+                           navigation_handle()->GetURL().spec().c_str()));

[nasko, 2017/03/28 19:58:01]

This code might also block navigations to data: URLs in <webview> tag for apps.
Is this something we intend on doing? I don't know if they are considered
renderer-initiated or browser-initiated, so an explicit test to confirm/deny
will be useful.

[meacer, 2017/03/28 20:44:35]

I've been looking at this in the last few days. IMO we shouldn't break webview
as there is no omnibox there and the risk of spoofing is different than drive by
browsing ending up on data URLs.

I haven't gotten to checking webview in this throttle yet -- currently I'm
trying to fix the renderer side check at FrameLoader.cpp with little success.
The problem is that there doesn't seem to be a way to get the topmost frame from
the renderer with a webview. In other words, I'd like to be able to do something
like this in RenderFrameImpl:

bool RenderFrameImpl::allowUnrestrictedDataUrlNavigations() {
  WebFrame topmostFrame = ... // (embedder's frame which has a chrome-extension
URL)
  if (GetExtensionOrAppByURL(topmostFrame.url()) // or something similar
    return true;
  // .. other checks, if any.
  return false;
}

I'm not sure if this is feasible though. Is it possible to get the webview's
embedder on the renderer side?

+    return CANCEL;
+  }
+  return PROCEED;
+}
+
+// static
+std::unique_ptr<NavigationThrottle>
+DataUrlNavigationThrottle::CreateThrottleForNavigation(
+    NavigationHandle* navigation_handle) {
+  return base::WrapUnique(new DataUrlNavigationThrottle(navigation_handle));

[nasko, 2017/03/28 19:58:01]

Does it make sense to install this throttle on all requests? We could
technically only create it for navigations to data: scheme. The only thing it
will not block is redirects to data: URLs from the web, but I think this is
already done elsewhere. 

Also, we allow webRequest API to redirect to data: URLs, so I would expect the
code as written to block those. If we move it to only install the throttle for
data: scheme, that would be avoided.

[meacer, 2017/03/30 20:43:55]

Changed it to install only if the data URL is potentially going to be blocked. I
manually checked and webRequest redirects to data URLs don't get handled by the
throttler even if it's always created (with PlzNavigate on).

+}
+
+}  // namespace content