Always use the caller's context in PepperTryCatchV8

content/renderer/pepper/pepper_try_catch.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/pepper/pepper_try_catch.h"
 
 #include "content/renderer/pepper/pepper_plugin_instance_impl.h"
 #include "gin/converter.h"
 #include "ppapi/shared_impl/ppapi_globals.h"
 #include "ppapi/shared_impl/var_tracker.h"
@@ skipping 52 matching lines @@
 
 PepperTryCatchV8::PepperTryCatchV8(
     PepperPluginInstanceImpl* instance,
     V8VarConverter::AllowObjectVars convert_objects,
     v8::Isolate* isolate)
     : PepperTryCatch(instance, convert_objects),
       exception_(PP_MakeUndefined()) {
   // Typically when using PepperTryCatchV8 we are passed an isolate. We verify
   // that this isolate is the same as the plugin isolate.
   DCHECK(isolate == instance_->GetIsolate());
-  // We assume we are already in the plugin context for PepperTryCatchV8.
-  DCHECK(GetContext() == isolate->GetCurrentContext());
+
+  // We switch to the plugin context. We may be in a different context if we
+  // called into the plugin from a frame that differs from the plugin's frame.

[dmichael (off chromium), 2014/09/09 21:38:06]

d'oh...

I don't think this is the right thing to do. If the context is different, I
think that implies that there's an extension using a background content script.
(Sorry I didn't remember this issue when reviewing before; it's subtle). We
probably should just use the caller's context in all cases (and maybe not enter
directly).

If the caller doesn't *have* a context, make them choose one explicitly. Cases
where the *plugin itself* initiates the interaction with JS, you might not have
a context, but it should be fine to use the "main world" one. When dispatching a
MessageEvent, we serialize the data part of the message, and it will be
reconstituted for each target event handler in their appropriate
"world"/context. So in that case the originating context doesn't matter. For
Flash...  we probably haven't thought hard about it, and there might not be a
good answer.

Anyway, we don't want a case where a background content script can get a v8
object from the wrong "world" or "isolate". I might explain this wrong, but I
think that can constitute a "capability leak". E.g., see
http://www.adambarth.com/papers/2009/barth-weinberger-song.pdf

The idea being if some extension accesses the plugin in the DOM, and they get
back a JavaScript object that is from the "main world" instead of the background
context's world, they might be able to access other bits of JavaScript objects
from the "main world", which is not supposed to be available to the extension.

+  v8::Handle<v8::Context> context = GetContext();
+  if (!context.IsEmpty())
+    context->Enter();
 }
 
 PepperTryCatchV8::~PepperTryCatchV8() {
+  v8::Handle<v8::Context> context = GetContext();
+  if (!context.IsEmpty())
+    context->Exit();
+
   ppapi::PpapiGlobals::Get()->GetVarTracker()->ReleaseVar(exception_);
 }
 
 bool PepperTryCatchV8::HasException() {
   return GetContext().IsEmpty() || exception_.type != PP_VARTYPE_UNDEFINED;
 }
 
 bool PepperTryCatchV8::ThrowException() {
   if (!HasException())
     return false;
@@ skipping 71 matching lines @@
   if (exception_is_set_) {
     NOTREACHED();
     return;
   }
   if (exception_)
     *exception_ = ppapi::StringVar::StringToPPVar(message, strlen(message));
   exception_is_set_ = true;
 }
 
 }  // namespace content