Extensions: Gate activeTab with file urls on having explicit file access.

When an extension is granted tab permission using activeTab (in response to say
clicking on its browser action), the extension is granted permission to the
tab's origin for the duration of the tab lifetime.

When this happens for a tab with a file url loaded, the extension gets
permission to the file scheme on the tab. This allows, for example, the
extension to read the contents of the page using apis like
chrome.tabs.executeScript. For file urls, this is not ideal since this does not
respect the "Allow access to file URLs" extension setting.

This CL changes this behavior, gating the access to the file scheme on the tab,
on the extension having explicit file access. This CL also adds extensive test
coverage for the behavior of tabs.executeScript on pages with file urls loaded
into them.

BUG=816685

Change-Id: I9175bb1883006fe594a93262c6825a962c285037
Reviewed-on: https://chromium-review.googlesource.com/994264
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548269}

chrome/browser/extensions/active_tab_apitest.cc

@@ -10,12 +10,14 @@
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/extensions/extension_util.h"
+#include "chrome/browser/sessions/session_tab_helper.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/test/test_utils.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/test_extension_registry_observer.h"
+#include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
 #include "extensions/test/extension_test_message_listener.h"
 #include "extensions/test/result_catcher.h"
@@ -100,8 +102,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, ActiveTab) {
 }
 
 // Tests the behavior of activeTab and its relation to an extension's ability to
-// xhr file urls.
-IN_PROC_BROWSER_TEST_F(ExtensionApiTest, XHRFileURLs) {
+// xhr file urls and inject scripts in file frames.
+IN_PROC_BROWSER_TEST_F(ExtensionApiTest, FileURLs) {
   ASSERT_TRUE(StartEmbeddedTestServer());
 
   ExtensionTestMessageListener background_page_ready("ready",
@@ -122,6 +124,11 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, XHRFileURLs) {
       req.onload = function() {
         if (req.responseText === 'Hello!')
           window.domAutomationController.send('true');
+
+        // Even for a successful request, the status code might be 0. Ensure
+        // that onloadend is not subsequently called if the request is
+        // successful.
+        req.onloadend = null;
       };
 
       // We track 'onloadend' to detect failures instead of 'onerror', since for
@@ -145,29 +152,90 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, XHRFileURLs) {
     return result == "true";
   };
 
+  auto can_script_tab = [this, &extension_id](int tab_id) {
+    constexpr char script[] = R"(
+      var tabID = %d;
+      chrome.tabs.executeScript(
+          tabID, {code: 'console.log("injected");'}, function() {
+            const expectedError = 'Cannot access contents of the page. ' +
+                'Extension manifest must request permission to access the ' +
+                'respective host.';
+
+            if (chrome.runtime.lastError &&
+                expectedError != chrome.runtime.lastError.message) {
+              window.domAutomationController.send(
+                  'unexpected error: ' + chrome.runtime.lastError.message);
+            } else {
+              window.domAutomationController.send(
+                  chrome.runtime.lastError ? 'false' : 'true');
+            }
+          });
+    )";
+
+    std::string result = ExecuteScriptInBackgroundPage(
+        extension_id, base::StringPrintf(script, tab_id));
+    EXPECT_TRUE(result == "true" || result == "false") << result;
+    return result == "true";
+  };
+
+  auto get_active_tab_id = [this]() {
+    SessionTabHelper* session_tab_helper = SessionTabHelper::FromWebContents(
+        browser()->tab_strip_model()->GetActiveWebContents());
+    if (!session_tab_helper) {
+      ADD_FAILURE();
+      return extension_misc::kUnknownTabId;
+    }
+    return session_tab_helper->session_id().id();
+  };
+
+  // Navigate to two file urls (the extension's manifest.json and background.js
+  // in this case).
+  GURL file_url_1 =
+      net::FilePathToFileURL(extension->path().AppendASCII("manifest.json"));
+  ui_test_utils::NavigateToURL(browser(), file_url_1);
+
+  // Assigned to |inactive_tab_id| since we open another foreground tab
+  // subsequently.
+  int inactive_tab_id = get_active_tab_id();
+  EXPECT_NE(extension_misc::kUnknownTabId, inactive_tab_id);
+
+  GURL file_url_2 =
+      net::FilePathToFileURL(extension->path().AppendASCII("background.js"));
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), file_url_2, WindowOpenDisposition::NEW_FOREGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+  int active_tab_id = get_active_tab_id();
+  EXPECT_NE(extension_misc::kUnknownTabId, active_tab_id);
+
+  EXPECT_NE(inactive_tab_id, active_tab_id);
+
   // By default the extension should have file access enabled. However, since it
   // does not have host permissions to the localhost on the file scheme, it
-  // should not be able to xhr file urls.
+  // should not be able to xhr file urls. For the same reason, it should not be
+  // able to execute script in the two tabs.
   EXPECT_TRUE(util::AllowFileAccess(extension_id, profile()));
   EXPECT_FALSE(can_xhr_file_urls());
-
-  // Navigate to a file url (the extension's manifest.json in this case).
-  GURL manifest_file_url =
-      net::FilePathToFileURL(extension->path().AppendASCII("manifest.json"));
-  ui_test_utils::NavigateToURL(browser(), manifest_file_url);
+  EXPECT_FALSE(can_script_tab(active_tab_id));
+  EXPECT_FALSE(can_script_tab(inactive_tab_id));
 
   // First don't grant the tab permission. Verify that the extension can't xhr
-  // file urls.
+  // file urls and can't script the two tabs.
   content::WebContents* web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
   ExtensionActionRunner::GetForWebContents(web_contents)
       ->RunAction(extension, false /*grant_tab_permissions*/);
   EXPECT_FALSE(can_xhr_file_urls());
+  EXPECT_FALSE(can_script_tab(active_tab_id));
+  EXPECT_FALSE(can_script_tab(inactive_tab_id));
 
-  // Now grant the tab permission. Ensure the extension can now xhr file urls.
+  // Now grant the tab permission. Ensure the extension can now xhr file urls
+  // and script the active tab. It should still not be able to script the
+  // background tab.
   ExtensionActionRunner::GetForWebContents(web_contents)
       ->RunAction(extension, true /*grant_tab_permissions*/);
   EXPECT_TRUE(can_xhr_file_urls());
+  EXPECT_TRUE(can_script_tab(active_tab_id));
+  EXPECT_FALSE(can_script_tab(inactive_tab_id));
 
   // Revoke extension's access to file urls. This will cause the extension to
   // reload, invalidating the |extension| pointer. Re-initialize the |extension|
@@ -183,10 +251,13 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, XHRFileURLs) {
   EXPECT_TRUE(background_page_ready.WaitUntilSatisfied());
 
   // Grant the tab permission for the active url to the extension. Ensure it
-  // still can't xhr file urls (since it does not have file access).
+  // still can't xhr file urls and script the active tab (since it does not
+  // have file access).
   ExtensionActionRunner::GetForWebContents(web_contents)
       ->RunAction(extension, true /*grant_tab_permissions*/);
   EXPECT_FALSE(can_xhr_file_urls());
+  EXPECT_FALSE(can_script_tab(active_tab_id));
+  EXPECT_FALSE(can_script_tab(inactive_tab_id));
 }
 
 }  // namespace

chrome/browser/extensions/active_tab_permission_granter.cc

@@ -8,6 +8,7 @@
 #include <vector>
 
 #include "chrome/browser/extensions/extension_action_runner.h"
+#include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/navigation_handle.h"
@@ -116,7 +117,14 @@ void ActiveTabPermissionGranter::GrantIfRequested(const Extension* extension) {
   if (should_grant_active_tab &&
       (permissions_data->HasWithheldImpliedAllHosts() ||
        permissions_data->HasAPIPermission(APIPermission::kActiveTab))) {
-    new_hosts.AddOrigin(UserScript::ValidUserScriptSchemes(),
+    // Gate activeTab for file urls on extensions having explicit access to file
+    // urls.
+    int valid_schemes = UserScript::ValidUserScriptSchemes();
+    if (!util::AllowFileAccess(extension->id(),
+                               web_contents()->GetBrowserContext())) {
+      valid_schemes &= ~URLPattern::SCHEME_FILE;
+    }
+    new_hosts.AddOrigin(valid_schemes,
                         web_contents()->GetVisibleURL().GetOrigin());
     new_apis.insert(APIPermission::kTab);
   }

chrome/browser/extensions/execute_script_apitest.cc

@@ -5,6 +5,8 @@
 #include "base/strings/string_number_conversions.h"
 #include "build/build_config.h"
 #include "chrome/browser/extensions/extension_apitest.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "net/base/filename_util.h"
 #include "net/dns/mock_host_resolver.h"
 
 class ExecuteScriptApiTest : public ExtensionApiTest {
@@ -110,6 +112,33 @@ IN_PROC_BROWSER_TEST_F(ExecuteScriptApiTest, RemovedFrames) {
   ASSERT_TRUE(RunExtensionTest("executescript/removed_frames")) << message_;
 }
 
+// Ensure that an extension can inject a script in a file frame provided it has
+// access to file urls enabled and the necessary host permissions.
+IN_PROC_BROWSER_TEST_F(ExecuteScriptApiTest, InjectScriptInFileFrameAllowed) {
+  // Navigate to a file url. The extension will subsequently try to inject a
+  // script into it.
+  base::FilePath test_file =
+      test_data_dir_.DirName().AppendASCII("test_file.txt");
+  ui_test_utils::NavigateToURL(browser(), net::FilePathToFileURL(test_file));
+
+  SetCustomArg("ALLOWED");
+  ASSERT_TRUE(RunExtensionTest("executescript/file_access")) << message_;
+}
+
+// Ensure that an extension can't inject a script in a file frame if it doesn't
+// have file access.
+IN_PROC_BROWSER_TEST_F(ExecuteScriptApiTest, InjectScriptInFileFrameDenied) {
+  // Navigate to a file url. The extension will subsequently try to inject a
+  // script into it.
+  base::FilePath test_file =
+      test_data_dir_.DirName().AppendASCII("test_file.txt");
+  ui_test_utils::NavigateToURL(browser(), net::FilePathToFileURL(test_file));
+
+  SetCustomArg("DENIED");
+  ASSERT_TRUE(RunExtensionTestNoFileAccess("executescript/file_access"))
+      << message_;
+}
+
 // If tests time out because it takes too long to run them, then this value can
 // be increased to split the DestructiveScriptTest tests in approximately equal
 // parts. Each part takes approximately the same time to run.

chrome/test/data/extensions/api_test/active_tab_file_urls/manifest.json

 {
   "manifest_version": 2,
-  "name": "Test xhr to file urls with activeTab",
+  "name": "Test script injection and xhr to file urls with activeTab",
   "version": "1.0",
   "browser_action": {
       "default_title": "activeTab"

chrome/test/data/extensions/api_test/executescript/file_access/background.js

+// Copyright 2018 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.
+
+chrome.test.getConfig(function(config) {
+  chrome.test.runTests([
+    function canExecuteScriptInFileURLs() {
+      chrome.test.assertTrue(
+          config.customArg === 'ALLOWED' || config.customArg === 'DENIED');
+      const canExecuteScript = config.customArg === 'ALLOWED';
+
+      // Only a single tab should be opened currently. A file url should be
+      // opened in it.
+      chrome.tabs.query({}, function(tabs) {
+        chrome.test.assertEq(1, tabs.length);
+        const url = new URL(tabs[0].url);
+        chrome.test.assertEq('file:', url.protocol);
+
+        // Inject a script into this tab.
+        chrome.tabs.executeScript(
+          tabs[0].id, { code: 'console.log("injected");' }, function() {
+            if (canExecuteScript) {
+              chrome.test.assertTrue(chrome.runtime.lastError === undefined);
+            } else {
+              const expectedError =
+                  `Cannot access contents of url "${tabs[0].url}". Extension `+
+                  `manifest must request permission to access this host.`;
+              chrome.test.assertEq(
+                  expectedError, chrome.runtime.lastError.message);
+            }
+            chrome.test.succeed();
+          });
+      });
+    }
+  ]);
+});

chrome/test/data/extensions/api_test/executescript/file_access/manifest.json

+{
+  "version": "1",
+  "manifest_version": 2,
+  "name": "Tests whether an extension can inject script into file frames",
+  "background": {
+    "scripts": ["background.js"]
+  },
+  "permissions": ["<all_urls>", "tabs"]
+}