Prevent bindings escalation on an existing NavigationEntry (attempt 2).

content/browser/web_contents/render_view_host_manager.cc

Index: content/browser/web_contents/render_view_host_manager.cc
diff --git a/content/browser/web_contents/render_view_host_manager.cc b/content/browser/web_contents/render_view_host_manager.cc
index 9a31ade8501e7a0c54ce9fbbfe6de8cfd013b12a..e8f8a54569c2454db4c004be1af67b10ebd09268 100644
--- a/content/browser/web_contents/render_view_host_manager.cc
+++ b/content/browser/web_contents/render_view_host_manager.cc
@@ -22,6 +22,7 @@
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
+#include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents_view.h"
 #include "content/public/browser/web_ui_controller.h"
 #include "content/public/browser/web_ui_controller_factory.h"
@@ -97,6 +98,23 @@ RenderWidgetHostView* RenderViewHostManager::GetRenderWidgetHostView() const {
   return render_view_host_->GetView();
 }
 
+void RenderViewHostManager::SetPendingWebUI(const NavigationEntryImpl& entry) {
+  pending_web_ui_.reset(
+      delegate_->CreateWebUIForRenderManager(entry.GetURL()));
+  pending_and_current_web_ui_.reset();
+
+  // If this is an existing NavigationEntry, make sure we're not granting it
+  // different bindings than it had before.  If so, note it and don't give it
+  // any bindings, to avoid a potential privilege escalation.
+  if (pending_web_ui_.get() &&
+      !entry.GetContentState().empty() &&

[nasko, 2013/02/13 23:44:02]

This seems redundant, since we already check the bindings on the entry. If an
entry is existing, its bindings shouldn't be kInvalidBindings.

[Charlie Reis, 2013/02/13 23:56:18]

Good point!  Updated.

+      entry.bindings() != NavigationEntryImpl::kInvalidBindings &&
+      pending_web_ui_->GetBindings() != entry.bindings()) {
+    RecordAction(UserMetricsAction("ProcessSwapBindingsMismatch_RVHM"));
+    pending_web_ui_.reset();
+  }
+}
+
 RenderViewHostImpl* RenderViewHostManager::Navigate(
     const NavigationEntryImpl& entry) {
   // Create a pending RenderViewHost. It will give us the one we should use
@@ -814,9 +832,7 @@ RenderViewHostImpl* RenderViewHostManager::UpdateRendererStateForNavigate(
     // It must also happen after the above conditional call to CancelPending(),
     // otherwise CancelPending may clear the pending_web_ui_ and the page will
     // not have its bindings set appropriately.
-    pending_web_ui_.reset(
-        delegate_->CreateWebUIForRenderManager(entry.GetURL()));
-    pending_and_current_web_ui_.reset();
+    SetPendingWebUI(entry);
 
     // Ensure that we have created RVHs for the new RVH's opener chain if
     // we are staying in the same BrowsingInstance. This allows the pending RVH
@@ -881,9 +897,7 @@ RenderViewHostImpl* RenderViewHostManager::UpdateRendererStateForNavigate(
       pending_web_ui_.reset();
       pending_and_current_web_ui_ = web_ui_->AsWeakPtr();
     } else {
-      pending_and_current_web_ui_.reset();
-      pending_web_ui_.reset(
-          delegate_->CreateWebUIForRenderManager(entry.GetURL()));
+      SetPendingWebUI(entry);
     }
 
     if (pending_web_ui() && render_view_host_->IsRenderViewLive())