Require user opt-in before allowing content script injection on file URLs.
BUG=47180
Review URL: http://codereview.chromium.org/2809034
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@50737 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/extensions/execute_code_in_tab_function.cc b/chrome/browser/extensions/execute_code_in_tab_function.cc
index c391db87..44de6bf 100644
--- a/chrome/browser/extensions/execute_code_in_tab_function.cc
+++ b/chrome/browser/extensions/execute_code_in_tab_function.cc
@@ -9,6 +9,7 @@
#include "chrome/browser/browser.h"
#include "chrome/browser/extensions/extension_tabs_module.h"
#include "chrome/browser/extensions/extension_tabs_module_constants.h"
+#include "chrome/browser/extensions/extensions_service.h"
#include "chrome/browser/extensions/file_reader.h"
#include "chrome/browser/tab_contents/tab_contents.h"
#include "chrome/common/extensions/extension.h"
@@ -66,7 +67,8 @@
// NOTE: This can give the wrong answer due to race conditions, but it is OK,
// we check again in the renderer.
- if (!GetExtension()->CanExecuteScriptOnHost(contents->GetURL(), &error_))
+ if (!profile()->GetExtensionsService()->CanExecuteScriptOnHost(
+ GetExtension(), contents->GetURL(), &error_))
return false;
if (script_info->HasKey(keys::kAllFramesKey)) {
diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc
index e73b2f6..0f9661c 100644
--- a/chrome/browser/extensions/extension_prefs.cc
+++ b/chrome/browser/extensions/extension_prefs.cc
@@ -67,6 +67,10 @@
// A preference that, if true, will allow this extension to run in incognito
// mode.
const wchar_t kPrefIncognitoEnabled[] = L"incognito";
+
+// A preference to control whether an extension is allowed to inject script in
+// pages with file URLs.
+const wchar_t kPrefAllowFileAccess[] = L"allowFileAccess";
}
////////////////////////////////////////////////////////////////////////////////
@@ -352,6 +356,17 @@
prefs_->SavePersistentPrefs();
}
+bool ExtensionPrefs::AllowFileAccess(const std::string& extension_id) {
+ return ReadExtensionPrefBoolean(extension_id, kPrefAllowFileAccess);
+}
+
+void ExtensionPrefs::SetAllowFileAccess(const std::string& extension_id,
+ bool allow) {
+ UpdateExtensionPref(extension_id, kPrefAllowFileAccess,
+ Value::CreateBooleanValue(allow));
+ prefs_->SavePersistentPrefs();
+}
+
void ExtensionPrefs::GetKilledExtensionIds(std::set<std::string>* killed_ids) {
const DictionaryValue* dict = prefs_->GetDictionary(kExtensionsPref);
if (!dict || dict->empty())
diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h
index 3f352e7ad..77487e9 100644
--- a/chrome/browser/extensions/extension_prefs.h
+++ b/chrome/browser/extensions/extension_prefs.h
@@ -105,6 +105,11 @@
bool IsIncognitoEnabled(const std::string& extension_id);
void SetIsIncognitoEnabled(const std::string& extension_id, bool enabled);
+ // Returns true if the user has chosen to allow this extension to inject
+ // scripts into pages with file URLs.
+ bool AllowFileAccess(const std::string& extension_id);
+ void SetAllowFileAccess(const std::string& extension_id, bool allow);
+
// Saves ExtensionInfo for each installed extension with the path to the
// version directory and the location. Blacklisted extensions won't be saved
// and neither will external extensions the user has explicitly uninstalled.
diff --git a/chrome/browser/extensions/extension_startup_browsertest.cc b/chrome/browser/extensions/extension_startup_browsertest.cc
index ed62a40..7566caa 100644
--- a/chrome/browser/extensions/extension_startup_browsertest.cc
+++ b/chrome/browser/extensions/extension_startup_browsertest.cc
@@ -63,6 +63,7 @@
if (!load_extension_.value().empty()) {
command_line->AppendSwitchWithValue(switches::kLoadExtension,
load_extension_.ToWStringHack());
+ command_line->AppendSwitch(switches::kDisableExtensionsFileAccessCheck);
}
}
@@ -143,6 +144,23 @@
TestInjection(true, true);
}
+// Tests that disallowing file access on an extension prevents it from injecting
+// script into a page with a file URL.
+IN_PROC_BROWSER_TEST_F(ExtensionsStartupTest, NoFileAccess) {
+ WaitForServicesToStart(4, true); // 1 component extension and 3 others.
+
+ ExtensionsService* service = browser()->profile()->GetExtensionsService();
+ for (size_t i = 0; i < service->extensions()->size(); ++i) {
+ if (service->AllowFileAccess(service->extensions()->at(i))) {
+ service->SetAllowFileAccess(service->extensions()->at(i), false);
+ ui_test_utils::WaitForNotification(
+ NotificationType::USER_SCRIPTS_UPDATED);
+ }
+ }
+
+ TestInjection(false, false);
+}
+
// ExtensionsLoadTest
// Ensures that we can startup the browser with --load-extension and see them
// run.
diff --git a/chrome/browser/extensions/extension_tabs_module.cc b/chrome/browser/extensions/extension_tabs_module.cc
index 71a8753..0f58360 100644
--- a/chrome/browser/extensions/extension_tabs_module.cc
+++ b/chrome/browser/extensions/extension_tabs_module.cc
@@ -646,7 +646,8 @@
// JavaScript URLs can do the same kinds of things as cross-origin XHR, so
// we need to check host permissions before allowing them.
if (url.SchemeIs(chrome::kJavaScriptScheme)) {
- if (!GetExtension()->CanExecuteScriptOnHost(contents->GetURL(), &error_))
+ if (!profile()->GetExtensionsService()->CanExecuteScriptOnHost(
+ GetExtension(), contents->GetURL(), &error_))
return false;
// TODO(aa): How does controller queue URLs? Is there any chance that this
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 99dcd9c..02a7366 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -38,6 +38,7 @@
#include "chrome/common/chrome_switches.h"
#include "chrome/common/extensions/extension.h"
#include "chrome/common/extensions/extension_constants.h"
+#include "chrome/common/extensions/extension_error_utils.h"
#include "chrome/common/extensions/extension_file_util.h"
#include "chrome/common/extensions/extension_l10n_util.h"
#include "chrome/common/notification_service.h"
@@ -675,6 +676,42 @@
NotifyExtensionLoaded(extension);
}
+bool ExtensionsService::AllowFileAccess(const Extension* extension) {
+ return (CommandLine::ForCurrentProcess()->HasSwitch(
+ switches::kDisableExtensionsFileAccessCheck) ||
+ extension_prefs_->AllowFileAccess(extension->id()));
+}
+
+void ExtensionsService::SetAllowFileAccess(Extension* extension, bool allow) {
+ extension_prefs_->SetAllowFileAccess(extension->id(), allow);
+ NotificationService::current()->Notify(
+ NotificationType::EXTENSION_USER_SCRIPTS_UPDATED,
+ Source<Profile>(profile_),
+ Details<Extension>(extension));
+}
+
+bool ExtensionsService::CanExecuteScriptOnHost(Extension* extension,
+ const GURL& url,
+ std::string* error) const {
+ // No extensions are allowed to execute script on the gallery because that
+ // would allow extensions to manipulate their own install pages.
+ if (url.host() == GURL(Extension::ChromeStoreURL()).host()) {
+ if (error)
+ *error = errors::kCannotScriptGallery;
+ return false;
+ }
+
+ if (extension->HasHostPermission(url))
+ return true;
+
+ if (error) {
+ *error = ExtensionErrorUtils::FormatErrorMessage(errors::kCannotAccessPage,
+ url.spec());
+ }
+
+ return false;
+}
+
void ExtensionsService::CheckForExternalUpdates() {
// This installs or updates externally provided extensions.
// TODO(aa): Why pass this list into the provider, why not just filter it
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index c7e0609..07bfa8b 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -155,6 +155,17 @@
bool IsIncognitoEnabled(const Extension* extension);
void SetIsIncognitoEnabled(Extension* extension, bool enabled);
+ // Whether this extension can inject scripts into pages with file URLs.
+ bool AllowFileAccess(const Extension* extension);
+ void SetAllowFileAccess(Extension* extension, bool allow);
+
+ // Returns true if the extension has permission to execute script on a
+ // particular host.
+ // TODO(aa): Also use this in the renderer, for normal content script
+ // injection. Currently, that has its own copy of this code.
+ bool CanExecuteScriptOnHost(Extension* extension,
+ const GURL& url, std::string* error) const;
+
const FilePath& install_directory() const { return install_directory_; }
// Initialize and start all installed extensions.
diff --git a/chrome/browser/extensions/extensions_ui.cc b/chrome/browser/extensions/extensions_ui.cc
index 9ad2e78..84a6f33c 100644
--- a/chrome/browser/extensions/extensions_ui.cc
+++ b/chrome/browser/extensions/extensions_ui.cc
@@ -125,6 +125,8 @@
l10n_util::GetString(IDS_EXTENSIONS_ENABLE));
localized_strings.SetString(L"enableIncognito",
l10n_util::GetString(IDS_EXTENSIONS_ENABLE_INCOGNITO));
+ localized_strings.SetString(L"allowFileAccess",
+ l10n_util::GetString(IDS_EXTENSIONS_ALLOW_FILE_ACCESS));
localized_strings.SetString(L"incognitoWarning",
l10n_util::GetString(IDS_EXTENSIONS_INCOGNITO_WARNING));
localized_strings.SetString(L"reload",
@@ -282,6 +284,8 @@
NewCallback(this, &ExtensionsDOMHandler::HandleEnableMessage));
dom_ui_->RegisterMessageCallback("enableIncognito",
NewCallback(this, &ExtensionsDOMHandler::HandleEnableIncognitoMessage));
+ dom_ui_->RegisterMessageCallback("allowFileAccess",
+ NewCallback(this, &ExtensionsDOMHandler::HandleAllowFileAccessMessage));
dom_ui_->RegisterMessageCallback("uninstall",
NewCallback(this, &ExtensionsDOMHandler::HandleUninstallMessage));
dom_ui_->RegisterMessageCallback("options",
@@ -493,6 +497,20 @@
ignore_notifications_ = false;
}
+void ExtensionsDOMHandler::HandleAllowFileAccessMessage(const Value* value) {
+ CHECK(value->IsType(Value::TYPE_LIST));
+ const ListValue* list = static_cast<const ListValue*>(value);
+ CHECK(list->GetSize() == 2);
+ std::string extension_id, allow_str;
+ CHECK(list->GetString(0, &extension_id));
+ CHECK(list->GetString(1, &allow_str));
+ Extension* extension = extensions_service_->GetExtensionById(extension_id,
+ true);
+ DCHECK(extension);
+
+ extensions_service_->SetAllowFileAccess(extension, allow_str == "true");
+}
+
void ExtensionsDOMHandler::HandleUninstallMessage(const Value* value) {
CHECK(value->IsType(Value::TYPE_LIST));
const ListValue* list = static_cast<const ListValue*>(value);
@@ -767,9 +785,23 @@
return script_data;
}
+static bool ExtensionWantsFileAccess(const Extension* extension) {
+ for (UserScriptList::const_iterator it = extension->content_scripts().begin();
+ it != extension->content_scripts().end(); ++it) {
+ for (UserScript::PatternList::const_iterator pattern =
+ it->url_patterns().begin();
+ pattern != it->url_patterns().end(); ++pattern) {
+ if (pattern->scheme() == chrome::kFileScheme)
+ return true;
+ }
+ }
+
+ return false;
+}
+
// Static
DictionaryValue* ExtensionsDOMHandler::CreateExtensionDetailValue(
- ExtensionsService* service, const Extension *extension,
+ ExtensionsService* service, const Extension* extension,
const std::vector<ExtensionPage>& pages, bool enabled) {
DictionaryValue* extension_data = new DictionaryValue();
@@ -780,6 +812,10 @@
extension_data->SetBoolean(L"enabled", enabled);
extension_data->SetBoolean(L"enabledIncognito",
service ? service->IsIncognitoEnabled(extension) : false);
+ extension_data->SetBoolean(L"wantsFileAccess",
+ ExtensionWantsFileAccess(extension));
+ extension_data->SetBoolean(L"allowFileAccess",
+ service ? service->AllowFileAccess(extension) : false);
extension_data->SetBoolean(L"allow_reload",
extension->location() == Extension::LOAD);
diff --git a/chrome/browser/extensions/extensions_ui.h b/chrome/browser/extensions/extensions_ui.h
index e85e586a0..120f08a 100644
--- a/chrome/browser/extensions/extensions_ui.h
+++ b/chrome/browser/extensions/extensions_ui.h
@@ -149,6 +149,9 @@
// Callback for "enableIncognito" message.
void HandleEnableIncognitoMessage(const Value* value);
+ // Callback for "allowFileAcces" message.
+ void HandleAllowFileAccessMessage(const Value* value);
+
// Callback for "uninstall" message.
void HandleUninstallMessage(const Value* value);
diff --git a/chrome/browser/extensions/user_script_master.cc b/chrome/browser/extensions/user_script_master.cc
index 608f9db..c651c2e2 100644
--- a/chrome/browser/extensions/user_script_master.cc
+++ b/chrome/browser/extensions/user_script_master.cc
@@ -306,6 +306,8 @@
Source<Profile>(profile_));
registrar_.Add(this, NotificationType::EXTENSION_UNLOADED,
Source<Profile>(profile_));
+ registrar_.Add(this, NotificationType::EXTENSION_USER_SCRIPTS_UPDATED,
+ Source<Profile>(profile_));
}
UserScriptMaster::~UserScriptMaster() {
@@ -348,11 +350,14 @@
Extension* extension = Details<Extension>(details).ptr();
bool incognito_enabled = profile_->GetExtensionsService()->
IsIncognitoEnabled(extension);
+ bool allow_file_access = profile_->GetExtensionsService()->
+ AllowFileAccess(extension);
const UserScriptList& scripts = extension->content_scripts();
for (UserScriptList::const_iterator iter = scripts.begin();
iter != scripts.end(); ++iter) {
lone_scripts_.push_back(*iter);
lone_scripts_.back().set_incognito_enabled(incognito_enabled);
+ lone_scripts_.back().set_allow_file_access(allow_file_access);
}
if (extensions_service_ready_)
StartScan();
@@ -375,6 +380,23 @@
break;
}
+ case NotificationType::EXTENSION_USER_SCRIPTS_UPDATED: {
+ Extension* extension = Details<Extension>(details).ptr();
+ UserScriptList new_lone_scripts;
+ bool incognito_enabled = profile_->GetExtensionsService()->
+ IsIncognitoEnabled(extension);
+ bool allow_file_access = profile_->GetExtensionsService()->
+ AllowFileAccess(extension);
+ for (UserScriptList::iterator iter = lone_scripts_.begin();
+ iter != lone_scripts_.end(); ++iter) {
+ if (iter->extension_id() == extension->id()) {
+ iter->set_incognito_enabled(incognito_enabled);
+ iter->set_allow_file_access(allow_file_access);
+ }
+ }
+ StartScan();
+ break;
+ }
default:
DCHECK(false);
