[Backport] Ignore certain messages in plugin broker process if they are not sent by the browser.

BUG=610600
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2069853002
Cr-Commit-Position: refs/heads/master@{#400495}
(cherry picked from commit 92cad45212731b59e74c8f2a2913691db3e9d770)

Review URL: https://codereview.chromium.org/2122313004 .
(CVE-2016-1706)

Change-Id: I3caa9f4432b1da7f1c1ed31ae95e222019f5439a
Reviewed-by: Michael Brüning <michael.bruning@qt.io>

9 files changed, 65 insertions, 21 deletions

diff --git a/chromium/content/browser/ppapi_plugin_process_host.cc b/chromium/content/browser/ppapi_plugin_process_host.cc
index b58c1d6e4d7..a116a531049 100644
--- a/chromium/content/browser/ppapi_plugin_process_host.cc
+++ b/chromium/content/browser/ppapi_plugin_process_host.cc
@@ -397,8 +397,12 @@ void PpapiPluginProcessHost::RequestPluginChannel(Client* client) {
   int renderer_child_id;
   client->GetPpapiChannelInfo(&process_handle, &renderer_child_id);
 
-  base::ProcessId process_id = (process_handle == base::kNullProcessHandle) ?
-      0 : base::GetProcId(process_handle);
+  base::ProcessId process_id = base::kNullProcessId;
+  if (process_handle != base::kNullProcessHandle) {
+    // This channel is not used by the browser itself.
+    process_id = base::GetProcId(process_handle);
+    CHECK_NE(base::kNullProcessId, process_id);
+  }
 
   // We can't send any sync messages from the browser because it might lead to
   // a hang. See the similar code in PluginProcessHost for more description.
diff --git a/chromium/content/browser/ppapi_plugin_process_host.h b/chromium/content/browser/ppapi_plugin_process_host.h
index e4c58a9b33b..c175700f3c0 100644
--- a/chromium/content/browser/ppapi_plugin_process_host.h
+++ b/chromium/content/browser/ppapi_plugin_process_host.h
@@ -34,6 +34,8 @@ class PpapiPluginProcessHost : public BrowserChildProcessHostDelegate,
   class Client {
    public:
     // Gets the information about the renderer that's requesting the channel.
+    // If |renderer_handle| is base::kNullProcessHandle, this channel is used by
+    // the browser itself.
     virtual void GetPpapiChannelInfo(base::ProcessHandle* renderer_handle,
                                      int* renderer_id) = 0;
 
diff --git a/chromium/content/browser/renderer_host/render_message_filter.cc b/chromium/content/browser/renderer_host/render_message_filter.cc
index 26cffacb94d..397157f7797 100644
--- a/chromium/content/browser/renderer_host/render_message_filter.cc
+++ b/chromium/content/browser/renderer_host/render_message_filter.cc
@@ -164,6 +164,9 @@ class OpenChannelToPpapiPluginCallback
 
   void GetPpapiChannelInfo(base::ProcessHandle* renderer_handle,
                            int* renderer_id) override {
+    // base::kNullProcessHandle indicates that the channel will be used by the
+    // browser itself. Make sure we never output that value here.
+    CHECK_NE(base::kNullProcessHandle, filter()->PeerHandle());
     *renderer_handle = filter()->PeerHandle();
     *renderer_id = filter()->render_process_id();
   }
@@ -197,6 +200,9 @@ class OpenChannelToPpapiBrokerCallback
 
   void GetPpapiChannelInfo(base::ProcessHandle* renderer_handle,
                            int* renderer_id) override {
+    // base::kNullProcessHandle indicates that the channel will be used by the
+    // browser itself. Make sure we never output that value here.
+    CHECK_NE(base::kNullProcessHandle, filter_->PeerHandle());
     *renderer_handle = filter_->PeerHandle();
     *renderer_id = filter_->render_process_id();
   }
diff --git a/chromium/content/ppapi_plugin/OWNERS b/chromium/content/ppapi_plugin/OWNERS
index 06209509b74..bf1820623e4 100644
--- a/chromium/content/ppapi_plugin/OWNERS
+++ b/chromium/content/ppapi_plugin/OWNERS
@@ -1,4 +1,5 @@
 piman@chromium.org
+yzshen@chromium.org
 
 # Mac Sandbox profiles.
 per-file *.sb=set noparent
diff --git a/chromium/content/ppapi_plugin/broker_process_dispatcher.cc b/chromium/content/ppapi_plugin/broker_process_dispatcher.cc
index 97d425043c8..90789149b54 100644
--- a/chromium/content/ppapi_plugin/broker_process_dispatcher.cc
+++ b/chromium/content/ppapi_plugin/broker_process_dispatcher.cc
@@ -6,6 +6,7 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/debug/dump_without_crashing.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/child/child_process.h"
@@ -75,12 +76,14 @@ void GetPermissionSettingsCallback(
 
 BrokerProcessDispatcher::BrokerProcessDispatcher(
     PP_GetInterface_Func get_plugin_interface,
-    PP_ConnectInstance_Func connect_instance)
+    PP_ConnectInstance_Func connect_instance,
+    bool peer_is_browser)
     : ppapi::proxy::BrokerSideDispatcher(connect_instance),
       get_plugin_interface_(get_plugin_interface),
       flash_browser_operations_1_3_(NULL),
       flash_browser_operations_1_2_(NULL),
-      flash_browser_operations_1_0_(NULL) {
+      flash_browser_operations_1_0_(NULL),
+      peer_is_browser_(peer_is_browser) {
   if (get_plugin_interface) {
     flash_browser_operations_1_0_ =
         static_cast<const PPP_Flash_BrowserOperations_1_0*>(
@@ -108,17 +111,36 @@ BrokerProcessDispatcher::~BrokerProcessDispatcher() {
 }
 
 bool BrokerProcessDispatcher::OnMessageReceived(const IPC::Message& msg) {
+  if (BrokerSideDispatcher::OnMessageReceived(msg))
+    return true;
+
+  if (!peer_is_browser_) {
+    // We might want to consider killing the peer instead is we see problems in
+    // the future.
+    if (msg.type() == PpapiMsg_GetSitesWithData::ID ||
+        msg.type() == PpapiMsg_ClearSiteData::ID ||
+        msg.type() == PpapiMsg_DeauthorizeContentLicenses::ID ||
+        msg.type() == PpapiMsg_GetPermissionSettings::ID ||
+        msg.type() == PpapiMsg_SetDefaultPermission::ID ||
+        msg.type() == PpapiMsg_SetSitePermission::ID) {
+      base::debug::DumpWithoutCrashing();
+    }
+    return false;
+  }
+
+  bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(BrokerProcessDispatcher, msg)
     IPC_MESSAGE_HANDLER(PpapiMsg_GetSitesWithData, OnGetSitesWithData)
     IPC_MESSAGE_HANDLER(PpapiMsg_ClearSiteData, OnClearSiteData)
     IPC_MESSAGE_HANDLER(PpapiMsg_DeauthorizeContentLicenses,
                         OnDeauthorizeContentLicenses)
-    IPC_MESSAGE_HANDLER(PpapiMsg_GetPermissionSettings, OnGetPermissionSettings)
+    IPC_MESSAGE_HANDLER(PpapiMsg_GetPermissionSettings,
+                        OnGetPermissionSettings)
     IPC_MESSAGE_HANDLER(PpapiMsg_SetDefaultPermission, OnSetDefaultPermission)
     IPC_MESSAGE_HANDLER(PpapiMsg_SetSitePermission, OnSetSitePermission)
-    IPC_MESSAGE_UNHANDLED(return BrokerSideDispatcher::OnMessageReceived(msg))
+    IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
-  return true;
+  return handled;
 }
 
 void BrokerProcessDispatcher::OnGetPermissionSettingsCompleted(
diff --git a/chromium/content/ppapi_plugin/broker_process_dispatcher.h b/chromium/content/ppapi_plugin/broker_process_dispatcher.h
index bbdbfdcc323..72eca9c7bcc 100644
--- a/chromium/content/ppapi_plugin/broker_process_dispatcher.h
+++ b/chromium/content/ppapi_plugin/broker_process_dispatcher.h
@@ -22,7 +22,8 @@ class BrokerProcessDispatcher
       public base::SupportsWeakPtr<BrokerProcessDispatcher> {
  public:
   BrokerProcessDispatcher(PP_GetInterface_Func get_plugin_interface,
-                          PP_ConnectInstance_Func connect_instance);
+                          PP_ConnectInstance_Func connect_instance,
+                          bool peer_is_browser);
   ~BrokerProcessDispatcher() override;
 
   // IPC::Listener overrides.
@@ -87,6 +88,8 @@ class BrokerProcessDispatcher
   const PPP_Flash_BrowserOperations_1_2* flash_browser_operations_1_2_;
   const PPP_Flash_BrowserOperations_1_0* flash_browser_operations_1_0_;
 
+  bool peer_is_browser_;
+
   DISALLOW_COPY_AND_ASSIGN(BrokerProcessDispatcher);
 };
 
diff --git a/chromium/content/ppapi_plugin/ppapi_thread.cc b/chromium/content/ppapi_plugin/ppapi_thread.cc
index c5b2338428a..51aa5af8439 100644
--- a/chromium/content/ppapi_plugin/ppapi_thread.cc
+++ b/chromium/content/ppapi_plugin/ppapi_thread.cc
@@ -453,8 +453,8 @@ void PpapiThread::OnCreateChannel(base::ProcessId renderer_pid,
   IPC::ChannelHandle channel_handle;
 
   if (!plugin_entry_points_.get_interface ||  // Plugin couldn't be loaded.
-      !SetupRendererChannel(renderer_pid, renderer_child_id, incognito,
-                            &channel_handle)) {
+      !SetupChannel(renderer_pid, renderer_child_id, incognito,
+                    &channel_handle)) {
     Send(new PpapiHostMsg_ChannelCreated(IPC::ChannelHandle()));
     return;
   }
@@ -486,10 +486,10 @@ void PpapiThread::OnHang() {
     base::PlatformThread::Sleep(base::TimeDelta::FromSeconds(1));
 }
 
-bool PpapiThread::SetupRendererChannel(base::ProcessId renderer_pid,
-                                       int renderer_child_id,
-                                       bool incognito,
-                                       IPC::ChannelHandle* handle) {
+bool PpapiThread::SetupChannel(base::ProcessId renderer_pid,
+                               int renderer_child_id,
+                               bool incognito,
+                               IPC::ChannelHandle* handle) {
   DCHECK(is_broker_ == (connect_instance_func_ != NULL));
   IPC::ChannelHandle plugin_handle;
   plugin_handle.name = IPC::Channel::GenerateVerifiedChannelID(
@@ -499,15 +499,17 @@ bool PpapiThread::SetupRendererChannel(base::ProcessId renderer_pid,
   ppapi::proxy::ProxyChannel* dispatcher = NULL;
   bool init_result = false;
   if (is_broker_) {
+    bool peer_is_browser = renderer_pid == base::kNullProcessId;
     BrokerProcessDispatcher* broker_dispatcher =
         new BrokerProcessDispatcher(plugin_entry_points_.get_interface,
-                                    connect_instance_func_);
+                                    connect_instance_func_, peer_is_browser);
     init_result = broker_dispatcher->InitBrokerWithChannel(this,
                                                            renderer_pid,
                                                            plugin_handle,
                                                            false);
     dispatcher = broker_dispatcher;
   } else {
+    DCHECK_NE(base::kNullProcessId, renderer_pid);
     PluginProcessDispatcher* plugin_dispatcher =
         new PluginProcessDispatcher(plugin_entry_points_.get_interface,
                                     permissions_,
diff --git a/chromium/content/ppapi_plugin/ppapi_thread.h b/chromium/content/ppapi_plugin/ppapi_thread.h
index 730af8589d9..9764b59ecea 100644
--- a/chromium/content/ppapi_plugin/ppapi_thread.h
+++ b/chromium/content/ppapi_plugin/ppapi_thread.h
@@ -108,12 +108,14 @@ class PpapiThread : public ChildThreadImpl,
   void OnCrash();
   void OnHang();
 
-  // Sets up the channel to the given renderer. On success, returns true and
-  // fills the given ChannelHandle with the information from the new channel.
-  bool SetupRendererChannel(base::ProcessId renderer_pid,
-                            int renderer_child_id,
-                            bool incognito,
-                            IPC::ChannelHandle* handle);
+  // Sets up the channel to the given renderer. If |renderer_pid| is
+  // base::kNullProcessId, the channel is set up to the browser. On success,
+  // returns true and fills the given ChannelHandle with the information from
+  // the new channel.
+  bool SetupChannel(base::ProcessId renderer_pid,
+                    int renderer_child_id,
+                    bool incognito,
+                    IPC::ChannelHandle* handle);
 
   // Sets up the name of the plugin for logging using the given path.
   void SavePluginName(const base::FilePath& path);
diff --git a/chromium/ppapi/proxy/ppapi_messages.h b/chromium/ppapi/proxy/ppapi_messages.h
index 2308a156fea..dfaa83a7f47 100644
--- a/chromium/ppapi/proxy/ppapi_messages.h
+++ b/chromium/ppapi/proxy/ppapi_messages.h
@@ -470,6 +470,8 @@ IPC_MESSAGE_CONTROL2(PpapiMsg_LoadPlugin,
 
 // Creates a channel to talk to a renderer. The plugin will respond with
 // PpapiHostMsg_ChannelCreated.
+// If |renderer_pid| is base::kNullProcessId, this is a channel used by the
+// browser itself.
 IPC_MESSAGE_CONTROL3(PpapiMsg_CreateChannel,
                      base::ProcessId /* renderer_pid */,
                      int /* renderer_child_id */,