Commit ecba6c37 authored by aa@chromium.org's avatar aa@chromium.org

Do not dereference potentially invalid frame pointer in extension event bindings.

I think this might fix bugs 93768 and 93774.

BUG=93768,93774

Review URL: http://codereview.chromium.org/7717019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@98390 0039d316-1c4b-4281-b951-d872f2087c98
parent b07f8f14
...@@ -133,7 +133,7 @@ ContextInfo::ContextInfo(v8::Persistent<v8::Context> context, ...@@ -133,7 +133,7 @@ ContextInfo::ContextInfo(v8::Persistent<v8::Context> context,
WebFrame* frame) WebFrame* frame)
: context(context), : context(context),
extension_id(extension_id), extension_id(extension_id),
frame(frame), unsafe_frame(frame),
num_connected_events(0) { num_connected_events(0) {
} }
......
...@@ -91,10 +91,10 @@ struct ContextInfo { ...@@ -91,10 +91,10 @@ struct ContextInfo {
// The extension ID this context is associated with. // The extension ID this context is associated with.
std::string extension_id; std::string extension_id;
// The frame the context is associated with. We can't always get this from // The frame the context is associated with. ContextInfo can outlive its
// WebFrame::frameForContext() (in particular as the the frame is navigating // frame, so this should not be dereferenced. It is stored only for use for
// or being destroyed). // comparison.
WebKit::WebFrame* frame; void* unsafe_frame;
// A count of the number of events that are listening in this context. When // A count of the number of events that are listening in this context. When
// this is zero, |context| will be a weak handle. // this is zero, |context| will be a weak handle.
......
...@@ -189,21 +189,15 @@ class ExtensionImpl : public ExtensionBase { ...@@ -189,21 +189,15 @@ class ExtensionImpl : public ExtensionBase {
// Returns true if the extension running in the given |context| has sufficient // Returns true if the extension running in the given |context| has sufficient
// permissions to access the data. // permissions to access the data.
static bool HasSufficientPermissions(ContextInfo* context, static bool HasSufficientPermissions(RenderView* render_view,
const GURL& event_url) { const GURL& event_url) {
v8::Context::Scope context_scope(context->context);
// During unit tests, we might be invoked without a v8 context. In these // During unit tests, we might be invoked without a v8 context. In these
// cases, we only allow empty event_urls and short-circuit before retrieving // cases, we only allow empty event_urls and short-circuit before retrieving
// the render view from the current context. // the render view from the current context.
if (!event_url.is_valid()) if (!event_url.is_valid())
return true; return true;
RenderView* renderview = bindings_utils::GetRenderViewForCurrentContext(); WebDocument document = render_view->webview()->mainFrame()->document();
if (!renderview)
return false;
WebDocument document = renderview->webview()->mainFrame()->document();
return GURL(document.url()).SchemeIs(chrome::kExtensionScheme) && return GURL(document.url()).SchemeIs(chrome::kExtensionScheme) &&
document.securityOrigin().canRequest(event_url); document.securityOrigin().canRequest(event_url);
} }
...@@ -364,7 +358,7 @@ void EventBindings::HandleContextDestroyed(WebFrame* frame) { ...@@ -364,7 +358,7 @@ void EventBindings::HandleContextDestroyed(WebFrame* frame) {
// itself might not be registered, but can still be a parent frame. // itself might not be registered, but can still be a parent frame.
for (ContextList::iterator it = GetContexts().begin(); for (ContextList::iterator it = GetContexts().begin();
it != GetContexts().end(); ) { it != GetContexts().end(); ) {
if ((*it)->frame == frame) { if ((*it)->unsafe_frame == frame) {
UnregisterContext(it, false); UnregisterContext(it, false);
// UnregisterContext will remove |it| from the list, but may also // UnregisterContext will remove |it| from the list, but may also
// modify the rest of the list as a result of calling into javascript. // modify the rest of the list as a result of calling into javascript.
...@@ -391,20 +385,25 @@ void EventBindings::CallFunction(const std::string& extension_id, ...@@ -391,20 +385,25 @@ void EventBindings::CallFunction(const std::string& extension_id,
V8ValueConverter converter; V8ValueConverter converter;
for (ContextList::iterator it = contexts.begin(); for (ContextList::iterator it = contexts.begin();
it != contexts.end(); ++it) { it != contexts.end(); ++it) {
if (render_view) { if ((*it)->context.IsEmpty())
RenderView* context_render_view = continue;
RenderView::FromWebView((*it)->frame->view());
if (render_view != context_render_view)
continue;
}
if (!extension_id.empty() && extension_id != (*it)->extension_id) if (!extension_id.empty() && extension_id != (*it)->extension_id)
continue; continue;
if ((*it)->context.IsEmpty()) WebFrame* context_frame = WebFrame::frameForContext((*it)->context);
if (!context_frame || !context_frame->view())
continue;
RenderView* context_render_view =
RenderView::FromWebView(context_frame->view());
if (!context_render_view)
continue;
if (render_view && render_view != context_render_view)
continue; continue;
if (!HasSufficientPermissions(it->get(), event_url)) if (!HasSufficientPermissions(context_render_view, event_url))
continue; continue;
v8::Local<v8::Context> context(*((*it)->context)); v8::Local<v8::Context> context(*((*it)->context));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment