The renderer and plugin processes can send over raw NPObjects valid in the other side's address

space. Basically the way this works is if an NPObject is marshaled over to the other side, an
NPObjectStub is created in the caller address space and a NPObjectProxy is created on the other side.
The NPObjectProxy is passed the raw NPObject pointer which is used as a cookie.

If the original NPObject needs to be passed back we pass the underlying NPObject saved in the NPObjectProxy.
The receiver does not validate whether this NPObject is valid before invoking on it.

While this is mostly fine, in the case of a compromised renderer invalid addresses could be passed back
to the plugin which would invoke on these addresses and crash.

Fix is to never pass raw object pointers across and just pass the corresponding routing id of the NPObjectStub.
The receiver validates this object by invoking a new method GetNPObjectListenerForRoute on the PluginChannelBase.
This method returns the corresponding NPObject listener for the routing id. We then retrieve the underlying NPObject
from the listener and use it.

The map of NPObjectListeners which is maintained by PluginChannelBase has been changed to hold NPObjectBase
pointers instead. NPObjectStub and NPObjectProxy implement the new NPObjectBase interface which provides 
methods to return the underlying NPObject and the IPC::Channel::Listener pointer.

Fixes bug http://code.google.com/p/chromium/issues/detail?id=31880

I verified with the steps outlined in the bug that this fix does address the underlying crash.

Bug=31880
Test=We need a framework to test PluginChannel and NPObjectProxy/Stub. Will add a test case for this
     once we have this in place.

Review URL: http://codereview.chromium.org/548046

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@36618 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 11 insertions, 20 deletions

diff --git a/chrome/common/plugin_messages.h b/chrome/common/plugin_messages.h
index eeea874..736e55e 100644
--- a/chrome/common/plugin_messages.h
+++ b/chrome/common/plugin_messages.h
@@ -83,11 +83,11 @@ enum NPVariant_ParamEnum {
   NPVARIANT_PARAM_STRING,
   // Used when when the NPObject is running in the caller's process, so we
   // create an NPObjectProxy in the other process.
-  NPVARIANT_PARAM_OBJECT_ROUTING_ID,
+  NPVARIANT_PARAM_SENDER_OBJECT_ROUTING_ID,
   // Used when the NPObject we're sending is running in the callee's process
   // (i.e. we have an NPObjectProxy for it).  In that case we want the callee
   // to just use the raw pointer.
-  NPVARIANT_PARAM_OBJECT_POINTER,
+  NPVARIANT_PARAM_RECEIVER_OBJECT_ROUTING_ID,
 };
 
 struct NPVariant_Param {
@@ -97,7 +97,6 @@ struct NPVariant_Param {
   double double_value;
   std::string string_value;
   int npobject_routing_id;
-  intptr_t npobject_pointer;
 };
 
 struct PluginMsg_UpdateGeometry_Param {
@@ -353,15 +352,12 @@ struct ParamTraits<NPVariant_Param> {
       WriteParam(m, p.double_value);
     } else if (p.type == NPVARIANT_PARAM_STRING) {
       WriteParam(m, p.string_value);
-    } else if (p.type == NPVARIANT_PARAM_OBJECT_ROUTING_ID) {
+    } else if (p.type == NPVARIANT_PARAM_SENDER_OBJECT_ROUTING_ID ||
+               p.type == NPVARIANT_PARAM_RECEIVER_OBJECT_ROUTING_ID) {
       // This is the routing id used to connect NPObjectProxy in the other
-      // process with NPObjectStub in this process.
+      // process with NPObjectStub in this process or to identify the raw
+      // npobject pointer to be used in the callee process.
       WriteParam(m, p.npobject_routing_id);
-      // The actual NPObject pointer, in case it's passed back to this process.
-      WriteParam(m, p.npobject_pointer);
-    } else if (p.type == NPVARIANT_PARAM_OBJECT_POINTER) {
-      // The NPObject resides in the other process, so just send its pointer.
-      WriteParam(m, p.npobject_pointer);
     } else {
       DCHECK(p.type == NPVARIANT_PARAM_VOID || p.type == NPVARIANT_PARAM_NULL);
     }
@@ -381,12 +377,9 @@ struct ParamTraits<NPVariant_Param> {
       result = ReadParam(m, iter, &r->double_value);
     } else if (r->type == NPVARIANT_PARAM_STRING) {
       result = ReadParam(m, iter, &r->string_value);
-    } else if (r->type == NPVARIANT_PARAM_OBJECT_ROUTING_ID) {
-      result =
-          ReadParam(m, iter, &r->npobject_routing_id) &&
-          ReadParam(m, iter, &r->npobject_pointer);
-    } else if (r->type == NPVARIANT_PARAM_OBJECT_POINTER) {
-      result = ReadParam(m, iter, &r->npobject_pointer);
+    } else if (r->type == NPVARIANT_PARAM_SENDER_OBJECT_ROUTING_ID ||
+               r->type == NPVARIANT_PARAM_RECEIVER_OBJECT_ROUTING_ID) {
+      result = ReadParam(m, iter, &r->npobject_routing_id);
     } else if ((r->type == NPVARIANT_PARAM_VOID) ||
                (r->type == NPVARIANT_PARAM_NULL)) {
       result = true;
@@ -405,11 +398,9 @@ struct ParamTraits<NPVariant_Param> {
       LogParam(p.double_value, l);
     } else if (p.type == NPVARIANT_PARAM_STRING) {
       LogParam(p.string_value, l);
-    } else if (p.type == NPVARIANT_PARAM_OBJECT_ROUTING_ID) {
+    } else if (p.type == NPVARIANT_PARAM_SENDER_OBJECT_ROUTING_ID ||
+               p.type == NPVARIANT_PARAM_RECEIVER_OBJECT_ROUTING_ID) {
       LogParam(p.npobject_routing_id, l);
-      LogParam(p.npobject_pointer, l);
-    } else if (p.type == NPVARIANT_PARAM_OBJECT_POINTER) {
-      LogParam(p.npobject_pointer, l);
     }
   }
 };