From 4d29dd615212c539463a4e4a02188aa34d06e96f Mon Sep 17 00:00:00 2001 From: raymes Date: Mon, 19 Jan 2015 12:56:15 -0800 Subject: Prevent UAF of RenderFrames from GuestViewContainer Current the RenderFrame a GuestViewContainer is associated with can be accessed even after it is destroyed. We should track the destruction with a RenderFrameObserver and avoid accesses in that case. BUG=449574 Review URL: https://codereview.chromium.org/822483007 Cr-Commit-Position: refs/heads/master@{#312143} --- .../guest_view/extensions_guest_view_container.cc | 6 ++++ .../renderer/guest_view/guest_view_container.cc | 35 ++++++++++++++++++++++ .../renderer/guest_view/guest_view_container.h | 11 +++++-- .../mime_handler_view_container.cc | 11 +++++++ 4 files changed, 61 insertions(+), 2 deletions(-) (limited to 'extensions') diff --git a/extensions/renderer/guest_view/extensions_guest_view_container.cc b/extensions/renderer/guest_view/extensions_guest_view_container.cc index ca5960d..cb480b9 100644 --- a/extensions/renderer/guest_view/extensions_guest_view_container.cc +++ b/extensions/renderer/guest_view/extensions_guest_view_container.cc @@ -57,6 +57,9 @@ ExtensionsGuestViewContainer::AttachRequest::~AttachRequest() { } void ExtensionsGuestViewContainer::AttachRequest::PerformRequest() { + if (!container()->render_frame()) + return; + // Step 1, send the attach params to extensions/. container()->render_frame()->Send( new ExtensionHostMsg_AttachGuest(container()->render_view_routing_id(), @@ -115,6 +118,9 @@ ExtensionsGuestViewContainer::DetachRequest::~DetachRequest() { } void ExtensionsGuestViewContainer::DetachRequest::PerformRequest() { + if (!container()->render_frame()) + return; + container()->render_frame()->DetachGuest(container()->element_instance_id()); } diff --git a/extensions/renderer/guest_view/guest_view_container.cc b/extensions/renderer/guest_view/guest_view_container.cc index 6991af4..5bb02bb 100644 --- a/extensions/renderer/guest_view/guest_view_container.cc +++ b/extensions/renderer/guest_view/guest_view_container.cc @@ -5,16 +5,47 @@ #include "extensions/renderer/guest_view/guest_view_container.h" #include "content/public/renderer/render_frame.h" +#include "content/public/renderer/render_frame_observer.h" #include "content/public/renderer/render_view.h" #include "extensions/common/extension_messages.h" #include "extensions/common/guest_view/guest_view_constants.h" namespace extensions { +namespace { + +class RenderFrameLifetimeObserver : public content::RenderFrameObserver { + public: + RenderFrameLifetimeObserver(GuestViewContainer* container, + content::RenderFrame* render_frame); + + // content::RenderFrameObserver overrides. + void OnDestruct() override; + + private: + GuestViewContainer* container_; + + DISALLOW_COPY_AND_ASSIGN(RenderFrameLifetimeObserver); +}; + +RenderFrameLifetimeObserver::RenderFrameLifetimeObserver( + GuestViewContainer* container, + content::RenderFrame* render_frame) + : content::RenderFrameObserver(render_frame), + container_(container) {} + +void RenderFrameLifetimeObserver::OnDestruct() { + container_->RenderFrameDestroyed(); +} + +} // namespace + GuestViewContainer::GuestViewContainer(content::RenderFrame* render_frame) : element_instance_id_(guestview::kInstanceIDNone), render_view_routing_id_(render_frame->GetRenderView()->GetRoutingID()), render_frame_(render_frame) { + render_frame_lifetime_observer_.reset( + new RenderFrameLifetimeObserver(this, render_frame_)); } GuestViewContainer::~GuestViewContainer() {} @@ -32,6 +63,10 @@ bool GuestViewContainer::HandlesMessage(const IPC::Message& msg) { } } +void GuestViewContainer::RenderFrameDestroyed() { + render_frame_ = nullptr; +} + void GuestViewContainer::SetElementInstanceID(int element_instance_id) { DCHECK_EQ(element_instance_id_, guestview::kInstanceIDNone); element_instance_id_ = element_instance_id; diff --git a/extensions/renderer/guest_view/guest_view_container.h b/extensions/renderer/guest_view/guest_view_container.h index 57a30da..036ece5 100644 --- a/extensions/renderer/guest_view/guest_view_container.h +++ b/extensions/renderer/guest_view/guest_view_container.h @@ -5,12 +5,16 @@ #ifndef EXTENSIONS_RENDERER_GUEST_VIEW_GUEST_VIEW_CONTAINER_H_ #define EXTENSIONS_RENDERER_GUEST_VIEW_GUEST_VIEW_CONTAINER_H_ +#include "base/memory/scoped_ptr.h" #include "content/public/renderer/browser_plugin_delegate.h" -#include "content/public/renderer/render_frame_observer.h" #include "ipc/ipc_message.h" namespace extensions { +namespace { +class RenderFrameLifetimeObserver; +} // namespace + class GuestViewContainer : public content::BrowserPluginDelegate { public: explicit GuestViewContainer(content::RenderFrame* render_frame); @@ -19,6 +23,8 @@ class GuestViewContainer : public content::BrowserPluginDelegate { // Queries whether GuestViewContainer is interested in the |message|. static bool HandlesMessage(const IPC::Message& message); + void RenderFrameDestroyed(); + // BrowserPluginDelegate implementation. void SetElementInstanceID(int element_instance_id) override; @@ -29,7 +35,8 @@ class GuestViewContainer : public content::BrowserPluginDelegate { private: int element_instance_id_; const int render_view_routing_id_; - content::RenderFrame* const render_frame_; + content::RenderFrame* render_frame_; + scoped_ptr render_frame_lifetime_observer_; DISALLOW_COPY_AND_ASSIGN(GuestViewContainer); }; diff --git a/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.cc b/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.cc index a7c19b2..a8663ea 100644 --- a/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.cc +++ b/extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.cc @@ -195,6 +195,10 @@ void MimeHandlerViewContainer::OnCreateMimeHandlerViewGuestACK( int element_instance_id) { DCHECK_NE(this->element_instance_id(), guestview::kInstanceIDNone); DCHECK_EQ(this->element_instance_id(), element_instance_id); + + if (!render_frame()) + return; + render_frame()->AttachGuest(element_instance_id); } @@ -207,6 +211,9 @@ void MimeHandlerViewContainer::OnGuestAttached(int /* unused */, void MimeHandlerViewContainer::OnMimeHandlerViewGuestOnLoadCompleted( int /* unused */) { + if (!render_frame()) + return; + guest_loaded_ = true; if (pending_messages_.empty()) return; @@ -230,6 +237,10 @@ void MimeHandlerViewContainer::CreateMimeHandlerViewGuest() { loader_.reset(); DCHECK_NE(element_instance_id(), guestview::kInstanceIDNone); + + if (!render_frame()) + return; + render_frame()->Send(new ExtensionHostMsg_CreateMimeHandlerViewGuest( render_frame()->GetRoutingID(), view_id_, element_instance_id())); } -- cgit v1.1