Fix a potential UaF in MimeHandlerViewContainerBase

The method MHVCB::GetEmbedderRenderFrame() is virtual *and* used inside the dtor
of MHVCB which is a mistake. Currently the references to MHVCB inside an
embedder frame do not get cleaned up after destruction (causing leaks inside
g_mime_handler_view_container_base_map).

This bug is also a potential root cause of some crashes which only show
themselves when the NetworkService is enabled (the corresponding codepath is
triggered when the feature is on).

Bug: 882645
Change-Id: I8c06184ac65054dc7e43d7582f99b2f7162280f0
Reviewed-on: https://chromium-review.googlesource.com/1225470Reviewed-by: James MacLean <wjmaclean@chromium.org>
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591341}

extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.cc

@@ -108,10 +108,6 @@ void MimeHandlerViewContainer::OnGuestAttached(int /* unused */,
   guest_proxy_routing_id_ = guest_proxy_routing_id;
 }
 
-content::RenderFrame* MimeHandlerViewContainer::GetEmbedderRenderFrame() const {
-  return render_frame();
-}
-
 void MimeHandlerViewContainer::CreateMimeHandlerViewGuestIfNecessary() {
   if (!element_size_.has_value())
     return;

extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container.h

@@ -60,7 +60,6 @@ class MimeHandlerViewContainer : public guest_view::GuestViewContainer,
 
  private:
   // MimeHandlerViewContainerBase override.
-  content::RenderFrame* GetEmbedderRenderFrame() const final;
   void CreateMimeHandlerViewGuestIfNecessary() final;
   blink::WebRemoteFrame* GetGuestProxyFrame() const final;
   int32_t GetInstanceId() const final;

extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_base.cc

@@ -170,6 +170,7 @@ MimeHandlerViewContainerBase::MimeHandlerViewContainerBase(
     : plugin_path_(info.path.MaybeAsASCII()),
       mime_type_(mime_type),
       original_url_(original_url),
+      embedder_render_frame_routing_id_(embedder_render_frame->GetRoutingID()),
       before_unload_control_binding_(this),
       weak_factory_(this) {
   DCHECK(!mime_type_.empty());
@@ -294,7 +295,8 @@ void MimeHandlerViewContainerBase::DidFinishLoading() {
 
 content::RenderFrame* MimeHandlerViewContainerBase::GetEmbedderRenderFrame()
     const {
-  return nullptr;
+  DCHECK_NE(embedder_render_frame_routing_id_, MSG_ROUTING_NONE);
+  return content::RenderFrame::FromRoutingID(embedder_render_frame_routing_id_);
 }
 
 void MimeHandlerViewContainerBase::CreateMimeHandlerViewGuestIfNecessary() {

extensions/renderer/guest_view/mime_handler_view/mime_handler_view_container_base.h

@@ -71,8 +71,6 @@ class MimeHandlerViewContainerBase : public blink::WebAssociatedURLLoaderClient,
   void DidFinishLoading() override;
 
  protected:
-  // Returns the frame which is embedding the corresponding plugin element.
-  virtual content::RenderFrame* GetEmbedderRenderFrame() const;
   virtual void CreateMimeHandlerViewGuestIfNecessary();
   virtual blink::WebRemoteFrame* GetGuestProxyFrame() const = 0;
   virtual int32_t GetInstanceId() const = 0;
@@ -102,6 +100,9 @@ class MimeHandlerViewContainerBase : public blink::WebAssociatedURLLoaderClient,
  private:
   class PluginResourceThrottle;
 
+  // Returns the frame which is embedding the corresponding plugin element.
+  content::RenderFrame* GetEmbedderRenderFrame() const;
+
   // Called for embedded plugins when network service is enabled. This is called
   // by the URLLoaderThrottle which intercepts the resource load, which is then
   // sent to the browser to be handed off to the plugin.
@@ -140,6 +141,9 @@ class MimeHandlerViewContainerBase : public blink::WebAssociatedURLLoaderClient,
   // has been called.
   bool guest_loaded_ = false;
 
+  // The routing ID of the frame which contains the plugin element.
+  const int32_t embedder_render_frame_routing_id_;
+
   mojo::Binding<mime_handler::BeforeUnloadControl>
       before_unload_control_binding_;
 

extensions/renderer/guest_view/mime_handler_view/mime_handler_view_frame_container.cc

@@ -55,8 +55,6 @@ MimeHandlerViewFrameContainer::MimeHandlerViewFrameContainer(
                                    plugin_info,
                                    mime_type,
                                    resource_url),
-      embedder_frame_(content::RenderFrame::FromWebFrame(
-          plugin_element.GetDocument().GetFrame())),
       plugin_element_(plugin_element),
       element_instance_id_(element_instance_id) {
   is_embedded_ = IsEmbedded();
@@ -72,12 +70,6 @@ MimeHandlerViewFrameContainer::MimeHandlerViewFrameContainer(
 
 MimeHandlerViewFrameContainer::~MimeHandlerViewFrameContainer() {}
 
-// Returns the frame which is embedding the corresponding plugin element.
-content::RenderFrame* MimeHandlerViewFrameContainer::GetEmbedderRenderFrame()
-    const {
-  return embedder_frame_;
-}
-
 void MimeHandlerViewFrameContainer::CreateMimeHandlerViewGuestIfNecessary() {
   if (auto* frame = GetContentFrame()) {
     plugin_frame_routing_id_ =

extensions/renderer/guest_view/mime_handler_view/mime_handler_view_frame_container.h

@@ -16,7 +16,6 @@ class WebFrame;
 }  // namespace blink
 
 namespace content {
-class RenderFrame;
 struct WebPluginInfo;
 }  // namespace content
 
@@ -42,7 +41,6 @@ class MimeHandlerViewFrameContainer : public MimeHandlerViewContainerBase {
   ~MimeHandlerViewFrameContainer() override;
 
   // MimeHandlerViewContainerBase overrides.
-  content::RenderFrame* GetEmbedderRenderFrame() const final;
   void CreateMimeHandlerViewGuestIfNecessary() final;
   blink::WebRemoteFrame* GetGuestProxyFrame() const final;
   int32_t GetInstanceId() const final;
@@ -64,7 +62,6 @@ class MimeHandlerViewFrameContainer : public MimeHandlerViewContainerBase {
 
   void OnMessageReceived(const IPC::Message& message);
 
-  content::RenderFrame* const embedder_frame_;
   blink::WebElement plugin_element_;
   const int32_t element_instance_id_;