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}

4 files changed, 61 insertions, 2 deletions

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<RenderFrameLifetimeObserver> 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()));
 }