Merge: 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

Committed: https://crrev.com/4d29dd615212c539463a4e4a02188aa34d06e96f
Cr-Commit-Position: refs/heads/master@{#312143}

Review URL: https://codereview.chromium.org/822483007

Cr-Commit-Position: refs/heads/master@{#312216}
(cherry picked from commit 569ce1bcb696473c117b8afdc15871183c0052ef)
TBR=raymes@chromium.org

Review URL: https://codereview.chromium.org/876983002

Cr-Commit-Position: refs/branch-heads/2272@{#124}
Cr-Branched-From: 827a380cfdb31aa54c8d56e63ce2c3fd8c3ba4d4-refs/heads/master@{#310958}

4 files changed, 56 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..3416428 100644
--- a/extensions/renderer/guest_view/guest_view_container.cc
+++ b/extensions/renderer/guest_view/guest_view_container.cc
@@ -5,16 +5,44 @@
 #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 {
 
+class GuestViewContainer::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);
+};
+
+GuestViewContainer::RenderFrameLifetimeObserver::RenderFrameLifetimeObserver(
+    GuestViewContainer* container,
+    content::RenderFrame* render_frame)
+    : content::RenderFrameObserver(render_frame),
+      container_(container) {}
+
+void GuestViewContainer::RenderFrameLifetimeObserver::OnDestruct() {
+  container_->RenderFrameDestroyed();
+}
+
 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 +60,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..df460cf 100644
--- a/extensions/renderer/guest_view/guest_view_container.h
+++ b/extensions/renderer/guest_view/guest_view_container.h
@@ -5,8 +5,8 @@
 #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 {
@@ -19,6 +19,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;
 
@@ -27,9 +29,12 @@ class GuestViewContainer : public content::BrowserPluginDelegate {
   content::RenderFrame* render_frame() const { return render_frame_; }
 
  private:
+  class RenderFrameLifetimeObserver;
+
   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 85f6930..4ae5060 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;
@@ -234,6 +241,10 @@ void MimeHandlerViewContainer::CreateMimeHandlerViewGuest() {
   DCHECK(!stream_url.spec().empty());
 
   DCHECK_NE(element_instance_id(), guestview::kInstanceIDNone);
+
+  if (!render_frame())
+    return;
+
   render_frame()->Send(new ExtensionHostMsg_CreateMimeHandlerViewGuest(
       render_frame()->GetRoutingID(), stream_url.spec(), original_url_.spec(),
       mime_type_, element_instance_id()));