Require a process ID when looking up RFHs by FrameTreeNode ID.

content/public/browser/web_contents.h

Index: content/public/browser/web_contents.h
diff --git a/content/public/browser/web_contents.h b/content/public/browser/web_contents.h
index b4ac5c96864391a7ee595cda8f85ebe9f9d587f8..5cce1884fb5388669ba7e27bcaf5f683a727fca9 100644
--- a/content/public/browser/web_contents.h
+++ b/content/public/browser/web_contents.h
@@ -245,9 +245,21 @@ class WebContents : public PageNavigator,
   virtual RenderFrameHost* GetFocusedFrame() = 0;
 
   // Returns the current RenderFrameHost for a given FrameTreeNode ID if it is
-  // part of this tab. See RenderFrameHost::GetFrameTreeNodeId for documentation
-  // on this ID.
-  virtual RenderFrameHost* FindFrameByFrameTreeNodeId(
+  // part of this tab. Returns nullptr if |process_id| does not match the
+  // current RenderFrameHost's process ID, to avoid security bugs where callers
+  // do not realize a cross-process navigation (and thus privilege change) has
+  // taken place. See RenderFrameHost::GetFrameTreeNodeId for documentation on
+  // frame_tree_node_id.
+  virtual RenderFrameHost* FindFrameByFrameTreeNodeId(int frame_tree_node_id,
+                                                      int process_id) = 0;

[ncarter (slow), 2017/05/01 22:15:32]

I agree with your choice of ordering these params, given how the function is
currently named. But both process_id and frame_tree_node_id want to be the first
parameter here, so I think there's some worry about the ordering being confused
here. At the very least this is one of those "look up the decl to double-check
the ordering."

Is there anything we can do to make this more obvious at the call site?

[Charlie Reis, 2017/05/01 22:36:51]

Heh, yeah, there is.  :)
https://bugs.chromium.org/p/chromium/issues/detail?id=715541#c4

Then again, I'm not super eager to convert all process IDs or even just FTN IDs
to gpu::IdType32<>, with potential bikeshedding around where IdType lives.  :) 
(If we did, though, we could make FTN IDs 64-bit while we're at it, and decide
what to do with extension APIs that expect them to be 32-bit.)

Not sure I have other suggestions, though, and I agree it's a bit unfortunate. 
You're right that I almost put process_id first (to be consistent with other
FromID methods that take a routing ID), but that felt more error prone here due
to the name.

I'm inclined to proceed for now, but let me know if you think we should tackle
something like the IdType conversion first.

+
+  // NOTE: This is generally unsafe to use. Use FindFrameByFrameTreeNodeId
+  // instead.
+  // Returns the current RenderFrameHost for a given FrameTreeNode ID if it is
+  // part of this tab. This may not match the caller's expectation, if a
+  // cross-process navigation (and thus privilege change) has taken place.
+  // See RenderFrameHost::GetFrameTreeNodeId for documentation on this ID.
+  virtual RenderFrameHost* UnsafeFindFrameByFrameTreeNodeId(
       int frame_tree_node_id) = 0;
 
   // Calls |on_frame| for each frame in the currently active view.