Don't compile functions in a context the caller doesn't have access to

Instead just return undefined

A side effect of this is that it's no longer possible to compile
functions in a detached context.

BUG=chromium:541703
R=verwaest@chromium.org,bmeurer@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_chromium_rel_ng

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

include/v8.h

@@ -5732,6 +5732,7 @@ class V8_EXPORT Isolate {
     kDecimalWithLeadingZeroInStrictMode = 32,
     kLegacyDateParser = 33,
     kDefineGetterOrSetterWouldThrow = 34,
+    kFunctionConstructorReturnedUndefined = 35,
 
     // If you add new values here, you'll also need to update Chromium's:
     // UseCounter.h, V8PerIsolateData.cpp, histograms.xml

src/builtins.cc

@@ -4723,13 +4723,23 @@ void Builtins::Generate_DatePrototypeGetUTCSeconds(MacroAssembler* masm) {
 namespace {
 
 // ES6 section 19.2.1.1.1 CreateDynamicFunction
-MaybeHandle<JSFunction> CreateDynamicFunction(Isolate* isolate,
-                                              BuiltinArguments args,
-                                              const char* token) {
+MaybeHandle<Object> CreateDynamicFunction(Isolate* isolate,
+                                          BuiltinArguments args,
+                                          const char* token) {
   // Compute number of arguments, ignoring the receiver.
   DCHECK_LE(1, args.length());
   int const argc = args.length() - 1;
 
+  Handle<JSFunction> target = args.target<JSFunction>();
+  Handle<JSObject> target_global_proxy(target->global_proxy(), isolate);
+
+  HandleScopeImplementer* impl = isolate->handle_scope_implementer();
+  if (!FLAG_allow_unsafe_function_constructor &&
+      !isolate->MayAccess(impl->LastEnteredContext(), target_global_proxy)) {
+    isolate->CountUsage(v8::Isolate::kFunctionConstructorReturnedUndefined);
+    return isolate->factory()->undefined_value();
+  }
+
   // Build the source string.
   Handle<String> source;
   {
@@ -4744,7 +4754,7 @@ MaybeHandle<JSFunction> CreateDynamicFunction(Isolate* isolate,
         Handle<String> param;
         ASSIGN_RETURN_ON_EXCEPTION(
             isolate, param, Object::ToString(isolate, args.at<Object>(i)),
-            JSFunction);
+            Object);
         param = String::Flatten(param);
         builder.AppendString(param);
         // If the formal parameters string include ) - an illegal
@@ -4769,37 +4779,35 @@ MaybeHandle<JSFunction> CreateDynamicFunction(Isolate* isolate,
       Handle<String> body;
       ASSIGN_RETURN_ON_EXCEPTION(
           isolate, body, Object::ToString(isolate, args.at<Object>(argc)),
-          JSFunction);
+          Object);
       builder.AppendString(body);
     }
     builder.AppendCString("\n})");
-    ASSIGN_RETURN_ON_EXCEPTION(isolate, source, builder.Finish(), JSFunction);
+    ASSIGN_RETURN_ON_EXCEPTION(isolate, source, builder.Finish(), Object);
 
     // The SyntaxError must be thrown after all the (observable) ToString
     // conversions are done.
     if (parenthesis_in_arg_string) {
       THROW_NEW_ERROR(isolate,
                       NewSyntaxError(MessageTemplate::kParenthesisInArgString),
-                      JSFunction);
+                      Object);
     }
   }
 
   // Compile the string in the constructor and not a helper so that errors to
   // come from here.
-  Handle<JSFunction> target = args.target<JSFunction>();
-  Handle<JSObject> target_global_proxy(target->global_proxy(), isolate);
   Handle<JSFunction> function;
   {
     ASSIGN_RETURN_ON_EXCEPTION(
         isolate, function,
         CompileString(handle(target->native_context(), isolate), source,
                       ONLY_SINGLE_FUNCTION_LITERAL),
-        JSFunction);
+        Object);
     Handle<Object> result;
     ASSIGN_RETURN_ON_EXCEPTION(
         isolate, result,
         Execution::Call(isolate, function, target_global_proxy, 0, nullptr),
-        JSFunction);
+        Object);
     function = Handle<JSFunction>::cast(result);
     function->shared()->set_name_should_print_as_anonymous(true);
   }
@@ -4818,7 +4826,7 @@ MaybeHandle<JSFunction> CreateDynamicFunction(Isolate* isolate,
     Handle<Map> initial_map;
     ASSIGN_RETURN_ON_EXCEPTION(
         isolate, initial_map,
-        JSFunction::GetDerivedMap(isolate, target, new_target), JSFunction);
+        JSFunction::GetDerivedMap(isolate, target, new_target), Object);
 
     Handle<SharedFunctionInfo> shared_info(function->shared(), isolate);
     Handle<Map> map = Map::AsLanguageMode(
@@ -4837,7 +4845,7 @@ MaybeHandle<JSFunction> CreateDynamicFunction(Isolate* isolate,
 // ES6 section 19.2.1.1 Function ( p1, p2, ... , pn, body )
 BUILTIN(FunctionConstructor) {
   HandleScope scope(isolate);
-  Handle<JSFunction> result;
+  Handle<Object> result;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
       isolate, result, CreateDynamicFunction(isolate, args, "function"));
   return *result;
@@ -4970,12 +4978,15 @@ BUILTIN(GeneratorFunctionConstructor) {
 
 BUILTIN(AsyncFunctionConstructor) {
   HandleScope scope(isolate);
-  Handle<JSFunction> func;
+  Handle<Object> maybe_func;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-      isolate, func, CreateDynamicFunction(isolate, args, "async function"));
+      isolate, maybe_func,
+      CreateDynamicFunction(isolate, args, "async function"));
+  if (!maybe_func->IsJSFunction()) return *maybe_func;
 
   // Do not lazily compute eval position for AsyncFunction, as they may not be
   // determined after the function is resumed.
+  Handle<JSFunction> func = Handle<JSFunction>::cast(maybe_func);
   Handle<Script> script = handle(Script::cast(func->shared()->script()));
   int position = script->GetEvalPosition();
   USE(position);

src/flag-definitions.h

@@ -588,6 +588,10 @@ DEFINE_BOOL(builtins_in_stack_traces, false,
             "show built-in functions in stack traces")
 DEFINE_BOOL(disable_native_files, false, "disable builtin natives files")
 
+// builtins.cc
+DEFINE_BOOL(allow_unsafe_function_constructor, false,
+            "allow invoking the function constructor without security checks")
+
 // builtins-ia32.cc
 DEFINE_BOOL(inline_new, true, "use fast inline allocation")
 

test/cctest/test-api.cc

@@ -10131,6 +10131,12 @@ static bool AccessAlwaysBlocked(Local<v8::Context> accessing_context,
   return false;
 }
 
+static bool AccessAlwaysAllowed(Local<v8::Context> accessing_context,
+                                Local<v8::Object> global,
+                                Local<v8::Value> data) {
+  i::PrintF("Access allowed.\n");
+  return true;
+}
 
 THREADED_TEST(AccessControlGetOwnPropertyNames) {
   v8::Isolate* isolate = CcTest::isolate();
@@ -25377,3 +25383,73 @@ THREADED_TEST(ImmutableProto) {
             ->Equals(context.local(), original_proto)
             .FromJust());
 }
+
+Local<v8::Context> call_eval_context;
+Local<v8::Function> call_eval_bound_function;
+
+static void CallEval(const v8::FunctionCallbackInfo<v8::Value>& args) {
+  v8::Context::Scope scope(call_eval_context);
+  args.GetReturnValue().Set(
+      call_eval_bound_function
+          ->Call(call_eval_context, call_eval_context->Global(), 0, NULL)
+          .ToLocalChecked());
+}
+
+TEST(CrossActivationEval) {
+  LocalContext env;
+  v8::Isolate* isolate = env->GetIsolate();
+  v8::HandleScope scope(isolate);
+  {
+    call_eval_context = v8::Context::New(isolate);
+    v8::Context::Scope scope(call_eval_context);
+    call_eval_bound_function =
+        Local<Function>::Cast(CompileRun("eval.bind(this, '1')"));
+  }
+  env->Global()
+      ->Set(env.local(), v8_str("CallEval"),
+            v8::FunctionTemplate::New(isolate, CallEval)
+                ->GetFunction(env.local())
+                .ToLocalChecked())
+      .FromJust();
+  Local<Value> result = CompileRun("CallEval();");
+  CHECK(result->IsInt32());
+  CHECK_EQ(1, result->Int32Value(env.local()).FromJust());
+}
+
+TEST(EvalInAccessCheckedContext) {
+  v8::Isolate* isolate = CcTest::isolate();
+  v8::HandleScope scope(isolate);
+
+  v8::Local<v8::ObjectTemplate> obj_template = v8::ObjectTemplate::New(isolate);
+
+  obj_template->SetAccessCheckCallback(AccessAlwaysAllowed);
+
+  v8::Local<Context> context0 = Context::New(isolate, NULL, obj_template);
+  v8::Local<Context> context1 = Context::New(isolate, NULL, obj_template);
+
+  Local<Value> foo = v8_str("foo");
+  Local<Value> bar = v8_str("bar");
+
+  // Set to different domains.
+  context0->SetSecurityToken(foo);
+  context1->SetSecurityToken(bar);
+
+  // Set up function in context0 that uses eval from context0.
+  context0->Enter();
+  v8::Local<v8::Value> fun = CompileRun(
+      "var x = 42;"
+      "(function() {"
+      "  var e = eval;"
+      "  return function(s) { return e(s); }"
+      "})()");
+  context0->Exit();
+
+  // Put the function into context1 and call it. Since the access check
+  // callback always returns true, the call succeeds even though the tokens
+  // are different.
+  context1->Enter();
+  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());
+  context1->Exit();
+}

test/mjsunit/cross-realm-filtering.js

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// Flags: --restricted-realms
+
 var realms = [Realm.current(), Realm.create()];
 
 // Check stack trace filtering across security contexts.
@@ -88,3 +90,77 @@ o = Realm.eval(realmIndex, "new f()");
 proto = Object.getPrototypeOf(o);
 assertFalse(proto === Object.prototype);
 assertTrue(proto === otherObject.prototype);
+
+// Check function constructor.
+var ctor_script = "Function";
+var ctor_a_script =
+    "(function() { return Function.apply(this, ['return 1;']); })";
+var ctor_b_script = "Function.bind(this, 'return 1;')";
+var ctor_c_script =
+    "(function() { return Function.call(this, 'return 1;'); })";
+Realm.shared = {
+  ctor_0 : Realm.eval(realms[0], ctor_script),
+  ctor_1 : Realm.eval(realms[1], ctor_script),
+  ctor_a_0 : Realm.eval(realms[0], ctor_a_script),
+  ctor_a_1 : Realm.eval(realms[1], ctor_a_script),
+  ctor_b_0 : Realm.eval(realms[0], ctor_b_script),
+  ctor_b_1 : Realm.eval(realms[1], ctor_b_script),
+  ctor_c_0 : Realm.eval(realms[0], ctor_c_script),
+  ctor_c_1 : Realm.eval(realms[1], ctor_c_script),
+}
+var script_0 = "                                                               \
+  var ctor_0 = Realm.shared.ctor_0;                                            \
+  Realm.shared.direct_0 = ctor_0('return 1');                                  \
+  Realm.shared.indirect_0 = (function() { return ctor_0('return 1;'); })();    \
+  Realm.shared.apply_0 = ctor_0.apply(this, ['return 1']);                     \
+  Realm.shared.bind_0 = ctor_0.bind(this, 'return 1')();                       \
+  Realm.shared.call_0 = ctor_0.call(this, 'return 1');                         \
+  Realm.shared.proxy_0 = new Proxy(ctor_0, {})('return 1');                    \
+  Realm.shared.reflect_0 = Reflect.apply(ctor_0, this, ['return 1']);          \
+  Realm.shared.a_0 = Realm.shared.ctor_a_0();                                  \
+  Realm.shared.b_0 = Realm.shared.ctor_b_0();                                  \
+  Realm.shared.c_0 = Realm.shared.ctor_c_0();                                  \
+";
+script = script_0 + script_0.replace(/_0/g, "_1");
+Realm.eval(realms[0], script);
+assertSame(1, Realm.shared.direct_0());
+assertSame(1, Realm.shared.indirect_0());
+assertSame(1, Realm.shared.apply_0());
+assertSame(1, Realm.shared.bind_0());
+assertSame(1, Realm.shared.call_0());
+assertSame(1, Realm.shared.proxy_0());
+assertSame(1, Realm.shared.reflect_0());
+assertSame(1, Realm.shared.a_0());
+assertSame(1, Realm.shared.b_0());
+assertSame(1, Realm.shared.c_0());
+assertSame(undefined, Realm.shared.direct_1);
+assertSame(undefined, Realm.shared.indirect_1);
+assertSame(undefined, Realm.shared.apply_1);
+assertSame(undefined, Realm.shared.bind_1);
+assertSame(undefined, Realm.shared.call_1);
+assertSame(undefined, Realm.shared.proxy_1);
+assertSame(undefined, Realm.shared.reflect_1);
+assertSame(undefined, Realm.shared.a_1);
+assertSame(undefined, Realm.shared.b_1);
+assertSame(undefined, Realm.shared.c_1);
+Realm.eval(realms[1], script);
+assertSame(undefined, Realm.shared.direct_0);
+assertSame(undefined, Realm.shared.indirect_0);
+assertSame(undefined, Realm.shared.apply_0);
+assertSame(undefined, Realm.shared.bind_0);
+assertSame(undefined, Realm.shared.call_0);
+assertSame(undefined, Realm.shared.proxy_0);
+assertSame(undefined, Realm.shared.reflect_0);
+assertSame(undefined, Realm.shared.a_0);
+assertSame(undefined, Realm.shared.b_0);
+assertSame(undefined, Realm.shared.c_0);
+assertSame(1, Realm.shared.direct_1());
+assertSame(1, Realm.shared.indirect_1());
+assertSame(1, Realm.shared.apply_1());
+assertSame(1, Realm.shared.bind_1());
+assertSame(1, Realm.shared.call_1());
+assertSame(1, Realm.shared.proxy_1());
+assertSame(1, Realm.shared.reflect_1());
+assertSame(1, Realm.shared.a_1());
+assertSame(1, Realm.shared.b_1());
+assertSame(1, Realm.shared.c_1());

test/mjsunit/es6/reflect-construct.js

 // Copyright 2014 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
+//
+// Flags: --allow-unsafe-function-constructor
 
 
 (function testReflectConstructArity() {