[Backport] Fix for CVE-2020-6438

Set tabs.executeScript() URLs to chrome-extension: scheme

Set the script URL for scripts executed via chrome.tabs.executeScript()
to use the chrome-extension: scheme, e.g.
chrome-extension://<id>/<path-to-script>, rather than the file URL.
This prevents referencing the filesystem in the URL, and is consistent
with content scripts that are statically specified in the manifest.

Add a regression test (that also tests the statically-defined content
script behavior). This entailed adding a new test utility,
WebContentsConsoleObserver, to track the messages sent to the console
for a given WebContents. This can replace ConsoleObserverDelegate in
the future.

Bug: 714617
Change-Id: I3de400e6dccf9f9a662824b4810bd52245cd4d62
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

6 files changed, 37 insertions, 40 deletions

diff --git a/chromium/extensions/browser/api/execute_code_function.cc b/chromium/extensions/browser/api/execute_code_function.cc
index 58d71034532..b9f374790ea 100644
--- a/chromium/extensions/browser/api/execute_code_function.cc
+++ b/chromium/extensions/browser/api/execute_code_function.cc
@@ -47,7 +47,7 @@ ExecuteCodeFunction::ExecuteCodeFunction() {
 ExecuteCodeFunction::~ExecuteCodeFunction() {
 }
 
-void ExecuteCodeFunction::GetFileURLAndMaybeLocalizeInBackground(
+void ExecuteCodeFunction::MaybeLocalizeInBackground(
     const std::string& extension_id,
     const base::FilePath& extension_path,
     const std::string& extension_default_locale,
@@ -57,11 +57,8 @@ void ExecuteCodeFunction::GetFileURLAndMaybeLocalizeInBackground(
   base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
                                                 base::BlockingType::MAY_BLOCK);
 
-  // TODO(devlin): FilePathToFileURL() doesn't need to be done on a blocking
-  // task runner, so we could do that on the UI thread and then avoid the hop
-  // if we don't need localization.
-  file_url_ = net::FilePathToFileURL(resource_.GetFilePath());
-
+  // TODO(devlin): Don't call the localization function if no localization is
+  // potentially required.
   if (!might_require_localization)
     return;
 
@@ -80,15 +77,15 @@ void ExecuteCodeFunction::GetFileURLAndMaybeLocalizeInBackground(
 }
 
 std::unique_ptr<std::string>
-ExecuteCodeFunction::GetFileURLAndLocalizeComponentResourceInBackground(
+ExecuteCodeFunction::LocalizeComponentResourceInBackground(
     std::unique_ptr<std::string> data,
     const std::string& extension_id,
     const base::FilePath& extension_path,
     const std::string& extension_default_locale,
     bool might_require_localization) {
-  GetFileURLAndMaybeLocalizeInBackground(
-      extension_id, extension_path, extension_default_locale,
-      might_require_localization, data.get());
+  MaybeLocalizeInBackground(extension_id, extension_path,
+                            extension_default_locale,
+                            might_require_localization, data.get());
 
   return data;
 }
@@ -170,7 +167,7 @@ bool ExecuteCodeFunction::Execute(const std::string& code_string,
       match_about_blank, run_at,
       IsWebView() ? ScriptExecutor::WEB_VIEW_PROCESS
                   : ScriptExecutor::DEFAULT_PROCESS,
-      GetWebViewSrc(), file_url_, user_gesture(), css_origin,
+      GetWebViewSrc(), script_url_, user_gesture(), css_origin,
       has_callback() ? ScriptExecutor::JSON_SERIALIZED_RESULT
                      : ScriptExecutor::NO_RESULT,
       base::Bind(&ExecuteCodeFunction::OnExecuteCodeFinished, this));
@@ -221,6 +218,8 @@ bool ExecuteCodeFunction::LoadFile(const std::string& file,
     return false;
   }
 
+  script_url_ = extension()->GetResourceURL(file);
+
   const std::string& extension_id = extension()->id();
   base::FilePath extension_path = extension()->path();
   std::string extension_default_locale;
@@ -247,19 +246,19 @@ bool ExecuteCodeFunction::LoadFile(const std::string& file,
         FROM_HERE,
         {base::ThreadPool(), base::MayBlock(),
          base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN},
-        base::BindOnce(&ExecuteCodeFunction::
-                           GetFileURLAndLocalizeComponentResourceInBackground,
-                       this, std::move(data), extension_id, extension_path,
-                       extension_default_locale, might_require_localization),
+        base::BindOnce(
+            &ExecuteCodeFunction::LocalizeComponentResourceInBackground, this,
+            std::move(data), extension_id, extension_path,
+            extension_default_locale,
+            might_require_localization),
         base::BindOnce(&ExecuteCodeFunction::DidLoadAndLocalizeFile, this,
                        resource_.relative_path().AsUTF8Unsafe(),
                        true /* We assume this call always succeeds */));
   } else {
     FileReader::OptionalFileSequenceTask get_file_and_l10n_callback =
-        base::BindOnce(
-            &ExecuteCodeFunction::GetFileURLAndMaybeLocalizeInBackground, this,
-            extension_id, extension_path, extension_default_locale,
-            might_require_localization);
+        base::BindOnce(&ExecuteCodeFunction::MaybeLocalizeInBackground, this,
+                       extension_id, extension_path, extension_default_locale,
+                       might_require_localization);
 
     auto file_reader = base::MakeRefCounted<FileReader>(
         resource_, std::move(get_file_and_l10n_callback),
diff --git a/chromium/extensions/browser/api/execute_code_function.h b/chromium/extensions/browser/api/execute_code_function.h
index 1971898ab57..b28c74067d0 100644
--- a/chromium/extensions/browser/api/execute_code_function.h
+++ b/chromium/extensions/browser/api/execute_code_function.h
@@ -79,23 +79,20 @@ class ExecuteCodeFunction : public ExtensionFunction {
                              const GURL& on_url,
                              const base::ListValue& result);
 
-  // Retrieves the file url for the given |extension_path| and optionally
-  // localizes |data|.
+  // Optionally localizes |data|.
   // Localization depends on whether |might_require_localization| was specified.
   // Only CSS file content needs to be localized.
-  void GetFileURLAndMaybeLocalizeInBackground(
+  void MaybeLocalizeInBackground(
       const std::string& extension_id,
       const base::FilePath& extension_path,
       const std::string& extension_default_locale,
       bool might_require_localization,
       std::string* data);
 
-  // Retrieves the file url for the given |extension_path| and optionally
-  // localizes |data|.
-  // Similar to GetFileURLAndMaybeLocalizeInBackground, but only applies
-  // to component extension resource.
-  std::unique_ptr<std::string>
-  GetFileURLAndLocalizeComponentResourceInBackground(
+  // Optionally localizes |data|.
+  // Similar to MaybeLocalizeInBackground, but only applies to component
+  // extension resources.
+  std::unique_ptr<std::string> LocalizeComponentResourceInBackground(
       std::unique_ptr<std::string> data,
       const std::string& extension_id,
       const base::FilePath& extension_path,
@@ -111,8 +108,9 @@ class ExecuteCodeFunction : public ExtensionFunction {
   // specified in JSON arguments.
   ExtensionResource resource_;
 
-  // The URL of the file being injected into the page.
-  GURL file_url_;
+  // The URL of the file being injected into the page, in the
+  // chrome-extension: scheme.
+  GURL script_url_;
 
   // The ID of the injection host.
   HostID host_id_;
diff --git a/chromium/extensions/browser/script_executor.cc b/chromium/extensions/browser/script_executor.cc
index f6266901ecf..8f7b64fdc18 100644
--- a/chromium/extensions/browser/script_executor.cc
+++ b/chromium/extensions/browser/script_executor.cc
@@ -39,10 +39,10 @@ const char kFrameRemoved[] = "The frame was removed.";
 // <host_id> is the host ID, and <digest> is an unspecified hash digest of the
 // file URL or the code string, respectively.
 const std::string GenerateInjectionKey(const HostID& host_id,
-                                       const GURL& file_url,
+                                       const GURL& script_url,
                                        const std::string& code) {
-  const std::string& source = file_url.is_valid() ? file_url.spec() : code;
-  return base::StringPrintf("%c%s%zu", file_url.is_valid() ? 'F' : 'C',
+  const std::string& source = script_url.is_valid() ? script_url.spec() : code;
+  return base::StringPrintf("%c%s%zu", script_url.is_valid() ? 'F' : 'C',
                             host_id.id().c_str(), base::FastHash(source));
 }
 
@@ -244,7 +244,7 @@ void ScriptExecutor::ExecuteScript(const HostID& host_id,
                                    UserScript::RunLocation run_at,
                                    ScriptExecutor::ProcessType process_type,
                                    const GURL& webview_src,
-                                   const GURL& file_url,
+                                   const GURL& script_url,
                                    bool user_gesture,
                                    base::Optional<CSSOrigin> css_origin,
                                    ScriptExecutor::ResultType result_type,
@@ -269,7 +269,7 @@ void ScriptExecutor::ExecuteScript(const HostID& host_id,
   params.run_at = run_at;
   params.is_web_view = (process_type == WEB_VIEW_PROCESS);
   params.webview_src = webview_src;
-  params.file_url = file_url;
+  params.script_url = script_url;
   params.wants_result = (result_type == JSON_SERIALIZED_RESULT);
   params.user_gesture = user_gesture;
   params.css_origin = css_origin;
@@ -277,7 +277,7 @@ void ScriptExecutor::ExecuteScript(const HostID& host_id,
   // Generate an injection key if this is a CSS injection from an extension
   // (i.e. tabs.insertCSS).
   if (host_id.type() == HostID::EXTENSIONS && script_type == CSS)
-    params.injection_key = GenerateInjectionKey(host_id, file_url, code);
+    params.injection_key = GenerateInjectionKey(host_id, script_url, code);
 
   // Handler handles IPCs and deletes itself on completion.
   new Handler(observer_, web_contents_, params, frame_scope, frame_id,
diff --git a/chromium/extensions/browser/script_executor.h b/chromium/extensions/browser/script_executor.h
index e19e905d716..7280493165b 100644
--- a/chromium/extensions/browser/script_executor.h
+++ b/chromium/extensions/browser/script_executor.h
@@ -102,7 +102,7 @@ class ScriptExecutor {
                      UserScript::RunLocation run_at,
                      ProcessType process_type,
                      const GURL& webview_src,
-                     const GURL& file_url,
+                     const GURL& script_url,
                      bool user_gesture,
                      base::Optional<CSSOrigin> css_origin,
                      ResultType result_type,
diff --git a/chromium/extensions/common/extension_messages.h b/chromium/extensions/common/extension_messages.h
index f207638c2fa..f73c4452e0a 100644
--- a/chromium/extensions/common/extension_messages.h
+++ b/chromium/extensions/common/extension_messages.h
@@ -183,8 +183,8 @@ IPC_STRUCT_BEGIN(ExtensionMsg_ExecuteCode_Params)
   // are examples of when this will be false.
   IPC_STRUCT_MEMBER(bool, wants_result)
 
-  // The URL of the file that was injected, if any.
-  IPC_STRUCT_MEMBER(GURL, file_url)
+  // The URL of the script that was injected, if any.
+  IPC_STRUCT_MEMBER(GURL, script_url)
 
   // Whether the code to be executed should be associated with a user gesture.
   IPC_STRUCT_MEMBER(bool, user_gesture)
diff --git a/chromium/extensions/renderer/programmatic_script_injector.cc b/chromium/extensions/renderer/programmatic_script_injector.cc
index 3f73988bbd3..123a5369404 100644
--- a/chromium/extensions/renderer/programmatic_script_injector.cc
+++ b/chromium/extensions/renderer/programmatic_script_injector.cc
@@ -110,7 +110,7 @@ std::vector<blink::WebScriptSource> ProgrammaticScriptInjector::GetJsSources(
 
   return std::vector<blink::WebScriptSource>(
       1, blink::WebScriptSource(blink::WebString::FromUTF8(params_->code),
-                                params_->file_url));
+                                params_->script_url));
 }
 
 std::vector<blink::WebString> ProgrammaticScriptInjector::GetCssSources(