Block renderer-initiated main frame navigations to data URLs

third_party/WebKit/Source/core/loader/FrameLoader.cpp

 /*
  * Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights
  * reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved.
  * (http://www.torchmobile.com/)
  * Copyright (C) 2008 Alp Toker <alp@atoker.com>
  * Copyright (C) Research In Motion Limited 2009. All rights reserved.
  * Copyright (C) 2011 Kris Jordan <krisjordan@gmail.com>
  * Copyright (C) 2011 Google Inc. All rights reserved.
@@ skipping 71 matching lines @@
 #include "core/svg/graphics/SVGImage.h"
 #include "core/xml/parser/XMLDocumentParser.h"
 #include "platform/InstanceCounters.h"
 #include "platform/PluginScriptForbiddenScope.h"
 #include "platform/ScriptForbiddenScope.h"
 #include "platform/UserGestureIndicator.h"
 #include "platform/instrumentation/tracing/TraceEvent.h"
 #include "platform/loader/fetch/ResourceFetcher.h"
 #include "platform/loader/fetch/ResourceRequest.h"
 #include "platform/network/HTTPParsers.h"
+#include "platform/network/NetworkUtils.h"
 #include "platform/scroll/ScrollAnimatorBase.h"
 #include "platform/weborigin/SchemeRegistry.h"
 #include "platform/weborigin/SecurityOrigin.h"
 #include "platform/weborigin/SecurityPolicy.h"
 #include "platform/weborigin/Suborigin.h"
 #include "platform/wtf/AutoReset.h"
 #include "platform/wtf/text/CString.h"
 #include "platform/wtf/text/WTFString.h"
 #include "public/platform/WebCachePolicy.h"
 #include "public/platform/WebURLRequest.h"
@@ skipping 639 matching lines @@
 
   KURL url = request.GetResourceRequest().Url();
   if (frame_->GetScriptController().ExecuteScriptIfJavaScriptURL(url, nullptr))
     return false;
 
   if (!request.OriginDocument()->GetSecurityOrigin()->CanDisplay(url)) {
     ReportLocalLoadFailed(frame_, url.ElidedString());
     return false;
   }
 
+  // Block renderer-initiated loads of data URLs in the top frame. If the mime
+  // type of the data URL is supported, the URL will eventually be rendered, so
+  // block it here. Otherwise, the load might be handled by a plugin or end up
+  // as a download, so allow it to let the embedder figure out what to do with
+  // it.
+  if (frame_->IsMainFrame() &&
+      !request.GetResourceRequest().IsSameDocumentNavigation() &&
+      !frame_->Client()->AllowContentInitiatedDataUrlNavigations(

[dcheng, 2017/04/15 01:09:38]

Sorry, to follow up on my other question: would it make sense to move this check
into DecidePolicyForNavigation() rather than adding another entry point for
navigation policy checks?

[meacer, 2017/04/17 22:21:58]

Quick update: I moved these to DecidePolicyForNavigation() but it broke a large
number of tests that are using RenderFrameTest::LoadHTML. Currently looking into
it.

[meacer, 2017/04/19 00:05:54]

Okay, I've been looking at this, and there are tests that I don't know how to
fix. The latest CL that does these checks in DecidePolicyForNavigation is at
https://codereview.chromium.org/2787123005/. So I'd like to land this as is.

+          request.OriginDocument()->Url()) &&
+      url.ProtocolIsData() && NetworkUtils::IsDataURLMimeTypeSupported(url)) {
+    ReportTopLevelNavigationFailed(frame_, url.ElidedString());
+    return false;
+  }
+
   if (!request.Form() && request.FrameName().IsEmpty())
     request.SetFrameName(frame_->GetDocument()->BaseTarget());
   return true;
 }
 
 static bool ShouldNavigateTargetFrame(NavigationPolicy policy) {
   switch (policy) {
     case kNavigationPolicyCurrentTab:
       return true;
 
@@ skipping 219 matching lines @@
 void FrameLoader::ReportLocalLoadFailed(LocalFrame* frame, const String& url) {
   DCHECK(!url.IsEmpty());
   if (!frame)
     return;
 
   frame->GetDocument()->AddConsoleMessage(
       ConsoleMessage::Create(kSecurityMessageSource, kErrorMessageLevel,
                              "Not allowed to load local resource: " + url));
 }
 
+void FrameLoader::ReportTopLevelNavigationFailed(LocalFrame* frame,
+                                                 const String& url) {
+  DCHECK(!url.IsEmpty());
+  if (!frame)

[dcheng, 2017/04/19 11:59:49]

Nit: just make this a non-static method (unlike ReportLocalLoadFailed, we don't
need to call it from elsewhere--and probably ReportLocalLoadFailed should just
be non-static, but that's out of scope here)

And frame (and frame_) if not static can never be null here.

[meacer, 2017/04/21 01:31:21]

This used to be called from outside FrameLoader but not anymore, so I just
inlined it into PrepareRequestForThisFrame.

+    return;
+
+  frame->GetDocument()->AddConsoleMessage(ConsoleMessage::Create(
+      kSecurityMessageSource, kErrorMessageLevel,
+      "Not allowed to top-level navigate to resource: " + url));
+}
+
 void FrameLoader::StopAllLoaders() {
   if (frame_->GetDocument()->PageDismissalEventBeingDispatched() !=
       Document::kNoDismissal)
     return;
 
   // If this method is called from within this method, infinite recursion can
   // occur (3442218). Avoid this.
   if (in_stop_all_loaders_)
     return;
 
@@ skipping 774 matching lines @@
   // TODO(japhet): This is needed because the browser process DCHECKs if the
   // first entry we commit in a new frame has replacement set. It's unclear
   // whether the DCHECK is right, investigate removing this special case.
   bool replace_current_item = load_type == kFrameLoadTypeReplaceCurrentItem &&
                               (!Opener() || !request.Url().IsEmpty());
   loader->SetReplacesCurrentHistoryItem(replace_current_item);
   return loader;
 }
 
 }  // namespace blink