DevTools extensions: validate devtools_page URLs

This ensures that the devtools_page URL has correct scheme and host,
both when loading the manifest and when pushing data to DevTools front-end.

Bug: 1059577
Change-Id: I69a7ccdccfae31781ead371a85d23df36f108665
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2118894
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753369}

chrome/browser/devtools/devtools_ui_bindings.cc

@@ -1420,8 +1420,14 @@ void DevToolsUIBindings::AddDevToolsExtensionsToClient() {
   for (const scoped_refptr<const extensions::Extension>& extension :
        registry->enabled_extensions()) {
     if (extensions::chrome_manifest_urls::GetDevToolsPage(extension.get())
-            .is_empty())
+            .is_empty()) {
       continue;
+    }
+    GURL url =
+        extensions::chrome_manifest_urls::GetDevToolsPage(extension.get());
+    const bool is_extension_url = url.SchemeIs(extensions::kExtensionScheme) &&
+                                  url.host_piece() == extension->id();
+    CHECK(is_extension_url || url.SchemeIsHTTPOrHTTPS());
 
     // Each devtools extension will need to be able to run in the devtools
     // process. Grant the devtools process the ability to request URLs from the
@@ -1432,10 +1438,7 @@ void DevToolsUIBindings::AddDevToolsExtensionsToClient() {
 
     std::unique_ptr<base::DictionaryValue> extension_info(
         new base::DictionaryValue());
-    extension_info->SetString(
-        "startPage",
-        extensions::chrome_manifest_urls::GetDevToolsPage(extension.get())
-            .spec());
+    extension_info->SetString("startPage", url.spec());
     extension_info->SetString("name", extension->name());
     extension_info->SetBoolean("exposeExperimentalAPIs",
                                extension->permissions_data()->HasAPIPermission(

chrome/common/extensions/chrome_manifest_url_handlers.cc

@@ -15,6 +15,7 @@
 #include "build/build_config.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/url_constants.h"
+#include "extensions/common/constants.h"
 #include "extensions/common/error_utils.h"
 #include "extensions/common/file_util.h"
 #include "extensions/common/manifest.h"
@@ -76,7 +77,15 @@ bool DevToolsPageHandler::Parse(Extension* extension, base::string16* error) {
     *error = base::ASCIIToUTF16(errors::kInvalidDevToolsPage);
     return false;
   }
-  manifest_url->url_ = extension->GetResourceURL(devtools_str);
+  GURL url = extension->GetResourceURL(devtools_str);
+  const bool is_extension_url =
+      url.SchemeIs(kExtensionScheme) && url.host_piece() == extension->id();
+  // TODO(caseq): using http(s) is unsupported and will be disabled in m83.
+  if (!is_extension_url && !url.SchemeIsHTTPOrHTTPS()) {
+    *error = base::ASCIIToUTF16(errors::kInvalidDevToolsPage);
+    return false;
+  }
+  manifest_url->url_ = std::move(url);
   extension->SetManifestData(keys::kDevToolsPage, std::move(manifest_url));
   PermissionsParser::AddAPIPermission(extension, APIPermission::kDevtools);
   return true;

chrome/common/extensions/docs/templates/articles/devtools.html

@@ -72,7 +72,8 @@ extension pages as panels and sidebars to the DevTools window using the
 
 <p class="note">The <code>devtools_page</code> field must point to an HTML page.
 This differs from the <code>background</code> field, used for specifying a background page,
-which lets you specify JavaScript files directly.</p>
+which lets you specify JavaScript files directly. The DevTools page must be
+local to your extension, so it is best to specify it using a relative URL.</p>
 
 <p>The <code>chrome.devtools.*</code> API modules are available only to the pages
 loaded within the DevTools window. Content scripts and other extension pages do not

chrome/common/extensions/manifest_tests/extension_manifests_devtools_unittest.cc

@@ -16,7 +16,17 @@ TEST_F(DevToolsPageManifestTest, DevToolsExtensions) {
   LoadAndExpectError("devtools_extension_url_invalid_type.json",
                      extensions::manifest_errors::kInvalidDevToolsPage);
 
+  LoadAndExpectError("devtools_extension_invalid_page_url.json",
+                     extensions::manifest_errors::kInvalidDevToolsPage);
+
+  // TODO(caseq): the implementation should be changed to produce failure
+  // with the manifest below.
   scoped_refptr<extensions::Extension> extension;
+  extension = LoadAndExpectSuccess("devtools_extension_page_url_https.json");
+  EXPECT_EQ("https://bad.example.com/dont_ever_do_this.html",
+            extensions::chrome_manifest_urls::GetDevToolsPage(extension.get())
+                .spec());
+
   extension = LoadAndExpectSuccess("devtools_extension.json");
   EXPECT_EQ(extension->url().spec() + "devtools.html",
             extensions::chrome_manifest_urls::GetDevToolsPage(extension.get())

chrome/test/data/extensions/manifest_tests/devtools_extension_invalid_page_url.json

+{
+  "name": "test",
+  "manifest_version": 2,
+  "version": "1",
+  "devtools_page": "data:text/html,hello"
+}

chrome/test/data/extensions/manifest_tests/devtools_extension_page_url_https.json

+{
+  "name": "test",
+  "manifest_version": 2,
+  "version": "1",
+  "devtools_page": "https://bad.example.com/dont_ever_do_this.html"
+}