Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(472)

Unified Diff: extensions/renderer/guest_view/guest_view_internal_custom_bindings.cc

Issue 2695633008: Re-check existence of GuestViewContainer before requesting attach. (Closed)
Patch Set: Remove unnecessary check. Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « components/guest_view/renderer/guest_view_container.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: extensions/renderer/guest_view/guest_view_internal_custom_bindings.cc
diff --git a/extensions/renderer/guest_view/guest_view_internal_custom_bindings.cc b/extensions/renderer/guest_view/guest_view_internal_custom_bindings.cc
index bec7d1c852016119b6a3df2e7c81ffbda0047867..77bc51f90115172134dee545dd4d04e3e0f5b7bd 100644
--- a/extensions/renderer/guest_view/guest_view_internal_custom_bindings.cc
+++ b/extensions/renderer/guest_view/guest_view_internal_custom_bindings.cc
@@ -15,6 +15,7 @@
#include "components/guest_view/renderer/iframe_guest_view_request.h"
#include "content/public/child/v8_value_converter.h"
#include "content/public/renderer/render_frame.h"
+#include "content/public/renderer/render_frame_observer.h"
#include "content/public/renderer/render_thread.h"
#include "content/public/renderer/render_view.h"
#include "extensions/common/extension.h"
@@ -57,6 +58,18 @@ content::RenderFrame* GetRenderFrame(v8::Handle<v8::Value> value) {
return content::RenderFrame::FromWebFrame(frame);
}
+class RenderFrameStatus : public content::RenderFrameObserver {
+ public:
+ explicit RenderFrameStatus(content::RenderFrame* render_frame)
+ : content::RenderFrameObserver(render_frame) {}
+ ~RenderFrameStatus() final {}
+
+ bool is_ok() { return render_frame() != nullptr; }
+
+ // RenderFrameObserver implementation.
+ void OnDestruct() final {}
+};
+
} // namespace
GuestViewInternalCustomBindings::GuestViewInternalCustomBindings(
@@ -144,6 +157,8 @@ void GuestViewInternalCustomBindings::AttachGuest(
// is invalid?
if (!guest_view_container)
return;
+ // Retain a weak pointer so we can easily test if the container goes away.
+ auto weak_ptr = guest_view_container->GetWeakPtr();
int guest_instance_id = args[1]->Int32Value();
@@ -155,6 +170,12 @@ void GuestViewInternalCustomBindings::AttachGuest(
params = base::DictionaryValue::From(std::move(params_as_value));
CHECK(params);
}
+ // We should be careful that some malicious JS in the GuestView's embedder
+ // hasn't destroyed |guest_view_container| during the enumeration of the
+ // properties of the guest's object during extraction of |params| above
+ // (see https://crbug.com/683523).
+ if (!weak_ptr)
+ return;
// Add flag to |params| to indicate that the element size is specified in
// logical units.
@@ -221,6 +242,12 @@ void GuestViewInternalCustomBindings::AttachIframeGuest(
int element_instance_id = args[0]->Int32Value();
int guest_instance_id = args[1]->Int32Value();
+ // Get the WebLocalFrame before (possibly) executing any user-space JS while
+ // getting the |params|. We track the status of the RenderFrame via an
+ // observer in case it is deleted during user code execution.
+ content::RenderFrame* render_frame = GetRenderFrame(args[3]);
+ RenderFrameStatus render_frame_status(render_frame);
+
std::unique_ptr<base::DictionaryValue> params;
{
std::unique_ptr<V8ValueConverter> converter(V8ValueConverter::create());
@@ -229,19 +256,19 @@ void GuestViewInternalCustomBindings::AttachIframeGuest(
params = base::DictionaryValue::From(std::move(params_as_value));
CHECK(params);
}
+ if (!render_frame_status.is_ok())
+ return;
- // Add flag to |params| to indicate that the element size is specified in
- // logical units.
- params->SetBoolean(guest_view::kElementSizeIsLogical, true);
-
- content::RenderFrame* render_frame = GetRenderFrame(args[3]);
blink::WebLocalFrame* frame = render_frame->GetWebFrame();
-
// Parent must exist.
blink::WebFrame* parent_frame = frame->parent();
DCHECK(parent_frame);
DCHECK(parent_frame->isWebLocalFrame());
+ // Add flag to |params| to indicate that the element size is specified in
+ // logical units.
+ params->SetBoolean(guest_view::kElementSizeIsLogical, true);
+
content::RenderFrame* embedder_parent_frame =
content::RenderFrame::FromWebFrame(parent_frame);
« no previous file with comments | « components/guest_view/renderer/guest_view_container.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698