Fix crashes caused by view-source: URLs for renderer debug URLs.

This change prevents a CHECK fail caused by renderer debug URLs making
their way deep into the navigation code and triggering a process lock
mismatch. There are several DCHECKs that indicate these types of URLs
should not make it into NavigationRequest code or lower layers, but
these are not enforced in release builds. This change adds a renderer
debug URL check to the set of other checks that run after the
view-source: URL rewriting logic has run. This prevents the navigation
from proceeding and prevents the URLs from getting down into the lower
levels of the navigation code.

Bug: 1070543
Change-Id: I6f2b54730164ae299c74bd80b77e9dbc15a87ad0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2157604Reviewed-by: Aaron Colwell <acolwell@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Auto-Submit: Aaron Colwell <acolwell@chromium.org>
Commit-Queue: Aaron Colwell <acolwell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#761136}

content/browser/frame_host/navigation_controller_impl.cc

@@ -268,6 +268,18 @@ bool IsValidURLForNavigation(bool is_main_frame,
     return false;
   }
 
+  // Reject renderer debug URLs because they should have been handled before
+  // we get to this point. This check handles renderer debug URLs
+  // that are inside a view-source: URL (e.g. view-source:chrome://kill) and
+  // provides defense-in-depth if a renderer debug URL manages to get here via
+  // some other path. We want to reject the navigation here so it doesn't
+  // violate assumptions in downstream code.
+  if (IsRendererDebugURL(dest_url)) {
+    LOG(WARNING) << "Refusing to load renderer debug URL: "
+                 << dest_url.possibly_invalid_spec();
+    return false;
+  }
+
   return true;
 }
 

content/browser/web_contents/web_contents_impl_unittest.cc

@@ -53,6 +53,7 @@
 #include "content/public/common/content_constants.h"
 #include "content/public/common/content_features.h"
 #include "content/public/common/url_constants.h"
+#include "content/public/common/url_utils.h"
 #include "content/public/test/fake_local_frame.h"
 #include "content/public/test/mock_render_process_host.h"
 #include "content/public/test/navigation_simulator.h"
@@ -581,6 +582,19 @@ TEST_F(WebContentsImplTest, NavigateToInvalidURL) {
   EXPECT_NE(nullptr, controller().GetPendingEntry());
 }
 
+// Test that we reject NavigateToEntry if the url is a renderer debug URL
+// inside a view-source: URL. This verifies that the navigation is not allowed
+// to proceed after the view-source: URL rewriting logic has run.
+TEST_F(WebContentsImplTest, NavigateToViewSourceRendererDebugURL) {
+  const GURL renderer_debug_url(kChromeUIKillURL);
+  const GURL view_source_debug_url("view-source:" + renderer_debug_url.spec());
+  EXPECT_TRUE(IsRendererDebugURL(renderer_debug_url));
+  EXPECT_FALSE(IsRendererDebugURL(view_source_debug_url));
+  controller().LoadURL(view_source_debug_url, Referrer(),
+                       ui::PAGE_TRANSITION_GENERATED, std::string());
+  EXPECT_EQ(nullptr, controller().GetPendingEntry());
+}
+
 // Test that navigating across a site boundary creates a new RenderViewHost
 // with a new SiteInstance.  Going back should do the same.
 TEST_F(WebContentsImplTest, CrossSiteBoundaries) {