Browser Plugin: Removed BrowserPluginWebContentsObserver (to be replaced by rewritten BrowserPlugi…

content/browser/renderer_host/render_view_host_impl.cc

Index: content/browser/renderer_host/render_view_host_impl.cc
diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
index 7ea514c0a281fdf6927159852ad4a320605b2578..de587377a694970c555cd2e21fed6e38a7d8d560 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -140,7 +140,6 @@ RenderViewHostImpl::RenderViewHostImpl(SiteInstance* instance,
       instance_(static_cast<SiteInstanceImpl*>(instance)),
       waiting_for_drag_context_response_(false),
       enabled_bindings_(0),
-      guest_(false),
       pending_request_id_(-1),
       navigations_suspended_(false),
       suspended_nav_message_(NULL),
@@ -205,7 +204,8 @@ content::SiteInstance* RenderViewHostImpl::GetSiteInstance() const {
 
 bool RenderViewHostImpl::CreateRenderView(const string16& frame_name,
                                           int opener_route_id,
-                                          int32 max_page_id) {
+                                          int32 max_page_id,
+                                          int embedder_process_id) {
   DCHECK(!IsRenderViewLive()) << "Creating view twice";
 
   // The process may (if we're sharing a process with another host that already
@@ -254,7 +254,11 @@ bool RenderViewHostImpl::CreateRenderView(const string16& frame_name,
       WebKit::WebScreenInfoFactory::screenInfo(
           gfx::NativeViewFromId(GetNativeViewId()));
 #endif
-  params.guest = guest_;
+
+  if (embedder_process_id != -1)
+    params.embedder_channel_name =
+        StringPrintf("%d.r%d", GetProcess()->GetID(), embedder_process_id);

[jam, 2012/05/15 00:05:06]

someone from the security team should take a look at this. we've had problems in
the past with processes connecting to processes that they shouldn't. in this
case it's a process that has the lowest privilege, but it would be good to
double check.

[jschuh, 2012/05/15 00:43:35]

Thanks John. It should probably be changed to something like:

Channel::GenerateVerifiedChannelID(
    StringPrintf("%d.r%d", GetProcess()->GetID(), embedder_process_id));

There's no overhead to speak of, and it avoids vulnerabilities for things like
privileged renderers or if the code changes in the future.

[Fady Samuel, 2012/05/15 02:09:15]

Hi Justin,

I don't quite understand the purpose of this function? This attaches a random ID
at the end of the string. In the renderer, I use this string as a key to decide
whether I have an existing channel to the embedder or whether I need to create a
new one. I can't do this if I have a random string. Thanks.

[jschuh, 2012/05/15 15:49:44]

The client gets a channel ID including the nonce via IPC or the command line.
When connecting, the client passes the random nonce to the server, which
verifies it. Otherwise the channel is vulnerable to hijack in the window between
creation and connection (one of the Pwnium exploits used that trick).

Also, it sounds like you're relying on process IDs not getting reused?
Unfortunately, that's not a safe assumption on Windows.

The GPU and plugin processes both do something similar to what you're
describing. IIRC they add an incrementing ID field for uniquely tracking the
channel in a map, and use GenerateVerifiedChannelID to generate the actual
channel.

+
   params.accessibility_mode =
       BrowserAccessibilityState::GetInstance()->IsAccessibleBrowser() ?
           AccessibilityModeComplete :