Do an access check before compiling code via eval()

Similarly to how we check whether the entered context has access to the target
context when invoking the function constructor, we should check the involved
contexts before invoking eval().

I forgot to add this in the initial CL that adds the check for the function
constructor. Move the code to a common location, and use it for the GlobalEval
builtin as well.

BUG=chromium:541703
R=verwaest@chromium.org

Review-Url: https://codereview.chromium.org/2199343002
Cr-Commit-Position: refs/heads/master@{#38277}

src/builtins/builtins-function.cc

@@ -13,22 +13,6 @@ namespace internal {
 
 namespace {
 
-bool AllowDynamicFunction(Isolate* isolate, Handle<JSFunction> target,
-                          Handle<JSObject> target_global_proxy) {
-  if (FLAG_allow_unsafe_function_constructor) return true;
-  HandleScopeImplementer* impl = isolate->handle_scope_implementer();
-  Handle<Context> responsible_context = impl->LastEnteredContext();
-  if (responsible_context.is_null()) {
-    responsible_context = impl->MicrotaskContext();
-    // TODO(jochen): Remove this.
-    if (responsible_context.is_null()) {
-      return true;
-    }
-  }
-  if (*responsible_context == target->context()) return true;
-  return isolate->MayAccess(responsible_context, target_global_proxy);
-}
-
 // ES6 section 19.2.1.1.1 CreateDynamicFunction
 MaybeHandle<Object> CreateDynamicFunction(Isolate* isolate,
                                           BuiltinArguments args,
@@ -40,7 +24,7 @@ MaybeHandle<Object> CreateDynamicFunction(Isolate* isolate,
   Handle<JSFunction> target = args.target<JSFunction>();
   Handle<JSObject> target_global_proxy(target->global_proxy(), isolate);
 
-  if (!AllowDynamicFunction(isolate, target, target_global_proxy)) {
+  if (!Builtins::AllowDynamicFunction(isolate, target, target_global_proxy)) {
     isolate->CountUsage(v8::Isolate::kFunctionConstructorReturnedUndefined);
     return isolate->factory()->undefined_value();
   }

src/builtins/builtins-global.cc

@@ -85,6 +85,10 @@ BUILTIN(GlobalEval) {
   Handle<JSFunction> target = args.target<JSFunction>();
   Handle<JSObject> target_global_proxy(target->global_proxy(), isolate);
   if (!x->IsString()) return *x;
+  if (!Builtins::AllowDynamicFunction(isolate, target, target_global_proxy)) {
+    isolate->CountUsage(v8::Isolate::kFunctionConstructorReturnedUndefined);
+    return isolate->heap()->undefined_value();
+  }
   Handle<JSFunction> function;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
       isolate, function, Compiler::GetFunctionFromString(

src/builtins/builtins.cc

@@ -206,5 +206,22 @@ const char* Builtins::name(int index) {
 BUILTIN_LIST_ALL(DEFINE_BUILTIN_ACCESSOR)
 #undef DEFINE_BUILTIN_ACCESSOR
 
+// static
+bool Builtins::AllowDynamicFunction(Isolate* isolate, Handle<JSFunction> target,
+                                    Handle<JSObject> target_global_proxy) {
+  if (FLAG_allow_unsafe_function_constructor) return true;
+  HandleScopeImplementer* impl = isolate->handle_scope_implementer();
+  Handle<Context> responsible_context = impl->LastEnteredContext();
+  if (responsible_context.is_null()) {
+    responsible_context = impl->MicrotaskContext();
+    // TODO(jochen): Remove this.
+    if (responsible_context.is_null()) {
+      return true;
+    }
+  }
+  if (*responsible_context == target->context()) return true;
+  return isolate->MayAccess(responsible_context, target_global_proxy);
+}
+
 }  // namespace internal
 }  // namespace v8

src/builtins/builtins.h

@@ -604,6 +604,9 @@ class Builtins {
   static void Generate_Adaptor(MacroAssembler* masm, Address builtin_address,
                                ExitFrameType exit_frame_type);
 
+  static bool AllowDynamicFunction(Isolate* isolate, Handle<JSFunction> target,
+                                   Handle<JSObject> target_global_proxy);
+
  private:
   Builtins();
 

test/cctest/test-api.cc

@@ -11676,6 +11676,9 @@ THREADED_TEST(EvalInDetachedGlobal) {
 
   v8::Local<Context> context0 = Context::New(isolate);
   v8::Local<Context> context1 = Context::New(isolate);
+  Local<String> token = v8_str("<security token>");
+  context0->SetSecurityToken(token);
+  context1->SetSecurityToken(token);
 
   // Set up function in context0 that uses eval from context0.
   context0->Enter();
@@ -11689,15 +11692,14 @@ THREADED_TEST(EvalInDetachedGlobal) {
 
   // Put the function into context1 and call it before and after
   // detaching the global.  Before detaching, the call succeeds and
-  // after detaching and exception is thrown.
+  // after detaching undefined is returned.
   context1->Enter();
   CHECK(context1->Global()->Set(context1, v8_str("fun"), fun).FromJust());
   v8::Local<v8::Value> x_value = CompileRun("fun('x')");
   CHECK_EQ(42, x_value->Int32Value(context1).FromJust());
   context0->DetachGlobal();
-  v8::TryCatch catcher(isolate);
   x_value = CompileRun("fun('x')");
-  CHECK_EQ(42, x_value->Int32Value(context1).FromJust());
+  CHECK(x_value->IsUndefined());
   context1->Exit();
 }