Check permissions for chrome.tabs.executeScript() in the
renderer just before injection to avoid races.
BUG=30937
Review URL: http://codereview.chromium.org/509032
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@35176 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 98757d2..5dc6690 100644
--- a/chrome/browser/extensions/execute_code_in_tab_function.cc
+++ b/chrome/browser/extensions/execute_code_in_tab_function.cc
@@ -68,6 +68,8 @@
DCHECK(browser);
DCHECK(contents);
+ // NOTE: This can give the wrong answer due to race conditions, but it is OK,
+ // we check again in the renderer.
if (!GetExtension()->CanAccessHost(contents->GetURL())) {
error_ = ExtensionErrorUtils::FormatErrorMessage(
keys::kCannotAccessPageError, contents->GetURL().spec());
@@ -135,6 +137,12 @@
return;
}
+ Extension* extension = GetExtension();
+ if (!extension) {
+ SendResponse(false);
+ return;
+ }
+
bool is_js_code = true;
std::string function_name = name();
if (function_name == TabsInsertCSSFunction::function_name()) {
@@ -145,8 +153,9 @@
registrar_.Add(this, NotificationType::TAB_CODE_EXECUTED,
NotificationService::AllSources());
AddRef(); // balanced in Observe()
- contents->ExecuteCode(request_id(), extension_id(), is_js_code,
- code_string, all_frames_);
+ contents->ExecuteCode(request_id(), extension->id(),
+ extension->host_permissions(), is_js_code, code_string,
+ all_frames_);
}
void ExecuteCodeInTabFunction::Observe(NotificationType type,
diff --git a/chrome/browser/extensions/execute_script_apitest.cc b/chrome/browser/extensions/execute_script_apitest.cc
index 902ffe6..07662b6c 100644
--- a/chrome/browser/extensions/execute_script_apitest.cc
+++ b/chrome/browser/extensions/execute_script_apitest.cc
@@ -7,10 +7,13 @@
// This test failed at times on the Vista dbg builder and has been marked as
// flaky for now. Bug http://code.google.com/p/chromium/issues/detail?id=28630
IN_PROC_BROWSER_TEST_F(ExtensionApiTest, FLAKY_ExecuteScript) {
- host_resolver()->AddRule("a.com", "127.0.0.1");
+ // We need a.com to be a little bit slow to trigger a race condition.
+ host_resolver()->AddRuleWithLatency("a.com", "127.0.0.1", 500);
host_resolver()->AddRule("b.com", "127.0.0.1");
+ host_resolver()->AddRule("c.com", "127.0.0.1");
StartHTTPServer();
ASSERT_TRUE(RunExtensionTest("executescript/basic")) << message_;
ASSERT_TRUE(RunExtensionTest("executescript/in_frame")) << message_;
+ ASSERT_TRUE(RunExtensionTest("executescript/permissions")) << message_;
}
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index 06801fd..39a25f2 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -815,12 +815,13 @@
}
void TabContents::ExecuteCode(int request_id, const std::string& extension_id,
+ const std::vector<URLPattern>& host_permissions,
bool is_js_code, const std::string& code_string,
bool all_frames) {
render_view_host()->Send(
- new ViewMsg_ExecuteCode(render_view_host()->routing_id(), request_id,
- extension_id, is_js_code, code_string,
- all_frames));
+ new ViewMsg_ExecuteCode(render_view_host()->routing_id(),
+ ViewMsg_ExecuteCode_Params(request_id, extension_id, host_permissions,
+ is_js_code, code_string, all_frames)));
}
void TabContents::PopupNotificationVisibilityChanged(bool visible) {
diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h
index 2a84b8c..112675d 100644
--- a/chrome/browser/tab_contents/tab_contents.h
+++ b/chrome/browser/tab_contents/tab_contents.h
@@ -32,6 +32,7 @@
#include "chrome/browser/tab_contents/navigation_entry.h"
#include "chrome/browser/tab_contents/page_navigator.h"
#include "chrome/browser/tab_contents/render_view_host_manager.h"
+#include "chrome/common/extensions/url_pattern.h"
#include "chrome/common/navigation_types.h"
#include "chrome/common/notification_registrar.h"
#include "chrome/common/property_bag.h"
@@ -368,6 +369,7 @@
// Execute code in this tab.
void ExecuteCode(int request_id, const std::string& extension_id,
+ const std::vector<URLPattern>& host_permissions,
bool is_js_code, const std::string& code_string,
bool all_frames);
diff --git a/chrome/common/render_messages.h b/chrome/common/render_messages.h
index c886522e..555ebd06 100644
--- a/chrome/common/render_messages.h
+++ b/chrome/common/render_messages.h
@@ -501,6 +501,39 @@
DOMStorageType storage_type_;
};
+// Allows an extension to execute code in a tab.
+struct ViewMsg_ExecuteCode_Params {
+ ViewMsg_ExecuteCode_Params(){}
+ ViewMsg_ExecuteCode_Params(int request_id, const std::string& extension_id,
+ const std::vector<URLPattern>& host_permissions,
+ bool is_javascript, const std::string& code,
+ bool all_frames)
+ : request_id(request_id), extension_id(extension_id),
+ host_permissions(host_permissions), is_javascript(is_javascript),
+ code(code), all_frames(all_frames) {
+ }
+
+ // The extension API request id, for responding.
+ int request_id;
+
+ // The ID of the requesting extension. To know which isolated world to
+ // execute the code inside of.
+ std::string extension_id;
+
+ // The host permissions of the requesting extension. So that we can check them
+ // right before injecting, to avoid any race conditions.
+ std::vector<URLPattern> host_permissions;
+
+ // Whether the code is JavaScript or CSS.
+ bool is_javascript;
+
+ // String of code to execute.
+ std::string code;
+
+ // Whether to inject into all frames, or only the root frame.
+ bool all_frames;
+};
+
namespace IPC {
template <>
@@ -2243,6 +2276,32 @@
}
};
+template<>
+struct ParamTraits<ViewMsg_ExecuteCode_Params> {
+ typedef ViewMsg_ExecuteCode_Params param_type;
+ static void Write(Message* m, const param_type& p) {
+ WriteParam(m, p.request_id);
+ WriteParam(m, p.extension_id);
+ WriteParam(m, p.host_permissions);
+ WriteParam(m, p.is_javascript);
+ WriteParam(m, p.code);
+ WriteParam(m, p.all_frames);
+ }
+
+ static bool Read(const Message* m, void** iter, param_type* p) {
+ return
+ ReadParam(m, iter, &p->request_id) &&
+ ReadParam(m, iter, &p->extension_id) &&
+ ReadParam(m, iter, &p->host_permissions) &&
+ ReadParam(m, iter, &p->is_javascript) &&
+ ReadParam(m, iter, &p->code) &&
+ ReadParam(m, iter, &p->all_frames);
+ }
+ static void Log(const param_type& p, std::wstring* l) {
+ l->append(L"<ViewMsg_ExecuteCode_Params>");
+ }
+};
+
} // namespace IPC
diff --git a/chrome/common/render_messages_internal.h b/chrome/common/render_messages_internal.h
index ca3e80b..453cdf38 100644
--- a/chrome/common/render_messages_internal.h
+++ b/chrome/common/render_messages_internal.h
@@ -745,15 +745,8 @@
ViewType::Type /* view_type */)
// Notification that renderer should run some JavaScript code.
- IPC_MESSAGE_ROUTED5(ViewMsg_ExecuteCode,
- int, /* request id */
- std::string, /* id of extension which runs the scripts */
- bool, /* It's true if the code is JavaScript; Otherwise
- the code is CSS text. */
- std::string, /* code would be executed */
- bool /* It's true if the code would be injected into all
- frames; Otherwise the code is only injected into
- top main frame */)
+ IPC_MESSAGE_ROUTED1(ViewMsg_ExecuteCode,
+ ViewMsg_ExecuteCode_Params)
// Returns a file handle
IPC_MESSAGE_CONTROL2(ViewMsg_DatabaseOpenFileResponse,
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index baac373..c4f7483 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -214,6 +214,16 @@
result->push_back(urls[i]);
}
+static bool UrlMatchesPermissions(
+ const GURL& url, const std::vector<URLPattern>& host_permissions) {
+ for (size_t i = 0; i < host_permissions.size(); ++i) {
+ if (host_permissions[i].MatchesUrl(url))
+ return true;
+ }
+
+ return false;
+}
+
///////////////////////////////////////////////////////////////////////////////
int32 RenderView::next_page_id_ = 1;
@@ -2358,10 +2368,9 @@
WebFrame* main_frame = webview()->mainFrame();
if (frame == main_frame) {
while (!pending_code_execution_queue_.empty()) {
- scoped_refptr<CodeExecutionInfo> info =
+ linked_ptr<ViewMsg_ExecuteCode_Params>& params =
pending_code_execution_queue_.front();
- ExecuteCodeImpl(main_frame, info->request_id, info->extension_id,
- info->is_js_code, info->code_string, info->all_frames);
+ ExecuteCodeImpl(main_frame, *params);
pending_code_execution_queue_.pop();
}
}
@@ -3807,57 +3816,53 @@
edit_commands_ = edit_commands;
}
-void RenderView::OnExecuteCode(int request_id, const std::string& extension_id,
- bool is_js_code,
- const std::string& code_string,
- bool all_frames) {
+void RenderView::OnExecuteCode(const ViewMsg_ExecuteCode_Params& params) {
WebFrame* main_frame = webview() ? webview()->mainFrame() : NULL;
if (!main_frame) {
- Send(new ViewMsg_ExecuteCodeFinished(routing_id_, request_id, false));
+ Send(new ViewMsg_ExecuteCodeFinished(routing_id_, params.request_id,
+ false));
return;
}
WebDataSource* ds = main_frame->dataSource();
NavigationState* navigation_state = NavigationState::FromDataSource(ds);
if (!navigation_state->user_script_idle_scheduler()->has_run()) {
- scoped_refptr<CodeExecutionInfo> info = new CodeExecutionInfo(
- request_id, extension_id, is_js_code, code_string, all_frames);
- pending_code_execution_queue_.push(info);
+ pending_code_execution_queue_.push(
+ linked_ptr<ViewMsg_ExecuteCode_Params>(
+ new ViewMsg_ExecuteCode_Params(params)));
return;
}
- ExecuteCodeImpl(main_frame, request_id, extension_id, is_js_code,
- code_string, all_frames);
+ ExecuteCodeImpl(main_frame, params);
}
void RenderView::ExecuteCodeImpl(WebFrame* frame,
- int request_id,
- const std::string& extension_id,
- bool is_js_code,
- const std::string& code_string,
- bool all_frames) {
+ const ViewMsg_ExecuteCode_Params& params) {
std::vector<WebFrame*> frame_vector;
frame_vector.push_back(frame);
- if (all_frames)
+ if (params.all_frames)
GetAllChildFrames(frame, &frame_vector);
for (std::vector<WebFrame*>::iterator frame_it = frame_vector.begin();
frame_it != frame_vector.end(); ++frame_it) {
WebFrame* frame = *frame_it;
- if (is_js_code) {
+ if (params.is_javascript) {
+ if (!UrlMatchesPermissions(frame->url(), params.host_permissions))
+ continue;
+
std::vector<WebScriptSource> sources;
sources.push_back(
- WebScriptSource(WebString::fromUTF8(code_string)));
- UserScriptSlave::InsertInitExtensionCode(&sources, extension_id);
+ WebScriptSource(WebString::fromUTF8(params.code)));
+ UserScriptSlave::InsertInitExtensionCode(&sources, params.extension_id);
frame->executeScriptInIsolatedWorld(
- UserScriptSlave::GetIsolatedWorldId(extension_id),
+ UserScriptSlave::GetIsolatedWorldId(params.extension_id),
&sources.front(), sources.size(), EXTENSION_GROUP_CONTENT_SCRIPTS);
} else {
- frame->insertStyleText(WebString::fromUTF8(code_string), WebString());
+ frame->insertStyleText(WebString::fromUTF8(params.code), WebString());
}
}
- Send(new ViewMsg_ExecuteCodeFinished(routing_id_, request_id, true));
+ Send(new ViewMsg_ExecuteCodeFinished(routing_id_, params.request_id, true));
}
void RenderView::Close() {
diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h
index c69a601..60bf6cb 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -16,6 +16,7 @@
#include "base/gfx/point.h"
#include "base/gfx/rect.h"
#include "base/id_map.h"
+#include "base/linked_ptr.h"
#include "base/shared_memory.h"
#include "base/timer.h"
#include "base/values.h"
@@ -25,6 +26,7 @@
#include "chrome/common/navigation_gesture.h"
#include "chrome/common/notification_type.h"
#include "chrome/common/page_zoom.h"
+#include "chrome/common/render_messages.h"
#include "chrome/common/renderer_preferences.h"
#include "chrome/common/view_types.h"
#include "chrome/renderer/automation/dom_automation_controller.h"
@@ -611,17 +613,9 @@
const WebKit::WebMediaPlayerAction& action);
void OnNotifyRendererViewType(ViewType::Type view_type);
void OnUpdateBrowserWindowId(int window_id);
- void OnExecuteCode(int request_id,
- const std::string& extension_id,
- bool is_js_code,
- const std::string& code_string,
- bool all_frames);
+ void OnExecuteCode(const ViewMsg_ExecuteCode_Params& params);
void ExecuteCodeImpl(WebKit::WebFrame* frame,
- int request_id,
- const std::string& extension_id,
- bool is_js_code,
- const std::string& code_string,
- bool all_frames);
+ const ViewMsg_ExecuteCode_Params& params);
void OnUpdateBackForwardListCount(int back_list_count,
int forward_list_count);
void OnGetAccessibilityInfo(
@@ -961,34 +955,8 @@
// Id number of browser window which RenderView is attached to.
int browser_window_id_;
- // If page is loading, we can't run code, just create CodeExecutionInfo
- // objects store pending execution information and delay the execution until
- // page is loaded.
- struct CodeExecutionInfo : public base::RefCounted<CodeExecutionInfo> {
- CodeExecutionInfo(int id_of_request, const std::string& id_of_extension,
- bool is_js, const std::string& code,
- bool inject_to_all_frames)
- : request_id(id_of_request),
- extension_id(id_of_extension),
- code_string(code),
- is_js_code(is_js),
- all_frames(inject_to_all_frames) {}
- int request_id;
-
- // The id of extension who issues the pending executeScript API call.
- std::string extension_id;
-
- // The code which would be executed.
- std::string code_string;
-
- // It's true if |code_string| is JavaScript; otherwise |code_string| is
- // CSS text.
- bool is_js_code;
- // It's true if the code_string would be injected into all frames.
- bool all_frames;
- };
-
- std::queue<scoped_refptr<CodeExecutionInfo> > pending_code_execution_queue_;
+ std::queue<linked_ptr<ViewMsg_ExecuteCode_Params> >
+ pending_code_execution_queue_;
// page id for the last navigation sent to the browser.
int32 last_top_level_navigation_page_id_;
diff --git a/chrome/test/data/extensions/api_test/executescript/permissions/empty.html b/chrome/test/data/extensions/api_test/executescript/permissions/empty.html
new file mode 100644
index 0000000..90531a4b
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/executescript/permissions/empty.html
@@ -0,0 +1,2 @@
+<html>
+</html>
diff --git a/chrome/test/data/extensions/api_test/executescript/permissions/frames.html b/chrome/test/data/extensions/api_test/executescript/permissions/frames.html
new file mode 100644
index 0000000..85ba47b3f
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/executescript/permissions/frames.html
@@ -0,0 +1,10 @@
+<html>
+<iframe src="http://a.com:1337/files/extensions/api_test/executescript/permissions/empty.html">
+</iframe>
+
+<iframe src="http://b.com:1337/files/extensions/api_test/executescript/permissions/empty.html">
+</iframe>
+
+<iframe src="http://c.com:1337/files/extensions/api_test/executescript/permissions/empty.html">
+</iframe>
+</html>
diff --git a/chrome/test/data/extensions/api_test/executescript/permissions/manifest.json b/chrome/test/data/extensions/api_test/executescript/permissions/manifest.json
new file mode 100644
index 0000000..50c7de35
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/executescript/permissions/manifest.json
@@ -0,0 +1,6 @@
+{
+ "version": "1.0.0.0",
+ "name": "Tests edge cases in chrome.tabs.executeScript() with permissions",
+ "background_page": "test.html",
+ "permissions": ["tabs", "http://a.com/", "http://b.com/"]
+}
diff --git a/chrome/test/data/extensions/api_test/executescript/permissions/script.js b/chrome/test/data/extensions/api_test/executescript/permissions/script.js
new file mode 100644
index 0000000..b73bce3
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/executescript/permissions/script.js
@@ -0,0 +1 @@
+chrome.extension.sendRequest(location.href);
diff --git a/chrome/test/data/extensions/api_test/executescript/permissions/test.html b/chrome/test/data/extensions/api_test/executescript/permissions/test.html
new file mode 100644
index 0000000..74c6e32
--- /dev/null
+++ b/chrome/test/data/extensions/api_test/executescript/permissions/test.html
@@ -0,0 +1,55 @@
+<script>
+var assertEq = chrome.test.assertEq;
+var assertTrue = chrome.test.assertTrue;
+var numReceivedRequests = 0;
+var relativePath = 'files/extensions/api_test/executescript/permissions/';
+var testFile = relativePath + 'empty.html';
+var testFileFrames = relativePath + 'frames.html';
+var onTabLoaded;
+
+chrome.extension.onRequest.addListener(function(req) {
+ numReceivedRequests++;
+});
+
+chrome.tabs.onUpdated.addListener(function(tabId, changeInfo, tab) {
+ if (tab.status == 'complete' && onTabLoaded)
+ onTabLoaded(tab);
+});
+
+chrome.test.runTests([
+ // Test a race that used to occur here (see bug 30937).
+ // Open a tab that we're not allowed to execute in (c.com), then navigate it
+ // to a tab we *are* allowed to execute in (a.com), then quickly run script in
+ // the tab before it navigates. It should appear to work (no error -- it could
+ // have been a developer mistake), but not actually do anything.
+ function() {
+ chrome.tabs.create({url: 'http://c.com:1337/' + testFile});
+ onTabLoaded = function(tab) {
+ onTabLoaded = null;
+ numReceivedRequests = 0;
+ chrome.tabs.update(tab.id, {url: 'http://a.com:1337/' + testFile});
+ chrome.tabs.executeScript(tab.id, {file: 'script.js'});
+ window.setTimeout(function() {
+ assertEq(0, numReceivedRequests);
+ chrome.test.runNextTest();
+ }, 4000);
+ };
+ },
+
+ // Inject into all frames. This should only affect frames we have access to.
+ // This page has three subframes, one each from a.com, b.com, and c.com. We
+ // have access to two of those, plus the root frame, so we should get three
+ // responses.
+ function() {
+ chrome.tabs.create({url: 'http://a.com:1337/' + testFileFrames});
+ onTabLoaded = function(tab) {
+ numReceivedRequests = 0;
+ chrome.tabs.executeScript(tab.id, {file: 'script.js', allFrames: true});
+ window.setTimeout(function() {
+ chrome.test.assertEq(3, numReceivedRequests);
+ chrome.test.runNextTest();
+ }, 4000);
+ };
+ }
+]);
+</script>
