Do security checks in the promise constructor

src/builtins/builtins-promise.cc

 // Copyright 2016 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.
 
 #include "src/builtins/builtins-promise.h"
 #include "src/builtins/builtins-constructor.h"
 #include "src/builtins/builtins-utils.h"
 #include "src/builtins/builtins.h"
 #include "src/code-factory.h"
 #include "src/code-stub-assembler.h"
@@ skipping 886 matching lines @@
                          Heap::kUndefinedValueRootIndex);
     StoreObjectFieldRoot(promise, JSPromise::kDeferredOnRejectOffset,
                          Heap::kUndefinedValueRootIndex);
     StoreObjectFieldRoot(promise, JSPromise::kFulfillReactionsOffset,
                          Heap::kUndefinedValueRootIndex);
     StoreObjectFieldRoot(promise, JSPromise::kRejectReactionsOffset,
                          Heap::kUndefinedValueRootIndex);
   }
 }
 
+void PromiseBuiltinsAssembler::BranchIfAccessCheckFailed(
+    Node* context, Node* promise_constructor, Node* executor,
+    Label* if_noaccess) {
+  Variable var_executor(this, MachineRepresentation::kTagged);
+  var_executor.Bind(executor);
+  Label has_access(this), call_runtime(this, Label::kDeferred);
+
+  // If executor is a bound function, load the bound function until we've
+  // reached an actual function.
+  Label found_function(this), loop_over_bound_function(this, &var_executor);
+  Goto(&loop_over_bound_function);
+  Bind(&loop_over_bound_function);

[gsathya, 2017/01/11 17:27:31]

i wonder if its better to duplicate the IsJSFunction check outside this block
and mark this block as deferred

[jochen (gone - plz use gerrit), 2017/01/11 18:09:57]

afaik that shouldn't make that much of a difference as there's not much going on
in the block. Adding ishell@ to double-check

[Igor Sheludko, 2017/01/12 08:31:11]

I think it's fine to avoid marking the block as deferred.

+  {
+    GotoIf(IsJSFunction(var_executor.value()), &found_function);
+    GotoUnless(IsJSBoundFunction(var_executor.value()), &call_runtime);

[Igor Sheludko, 2017/01/12 08:31:11]

Here you are loading map and then instance type second time. Please fix this.

+    var_executor.Bind(LoadObjectField(
+        var_executor.value(), JSBoundFunction::kBoundTargetFunctionOffset));
+    Goto(&loop_over_bound_function);
+  }
+
+  // Load the context from the function and compare it to the Promise
+  // constructor's context. If they match, everything is fine, otherwise, bail
+  // out to the runtime.
+  Bind(&found_function);
+  {
+    Node* native_context = LoadNativeContext(context);

[gsathya, 2017/01/11 17:27:31]

this can be an argument to this function, no need to load the native context
twice in the fast path

[jochen (gone - plz use gerrit), 2017/01/11 18:09:57]

done

+    Node* function_context =
+        LoadObjectField(var_executor.value(), JSFunction::kContextOffset);
+    Node* native_function_context = LoadNativeContext(function_context);
+    Branch(WordEqual(native_context, native_function_context), &has_access,
+           &call_runtime);
+  }
+
+  Bind(&call_runtime);
+  {
+    Branch(WordEqual(CallRuntime(Runtime::kAllowDynamicFunction, context,
+                                 promise_constructor),
+                     BooleanConstant(true)),
+           &has_access, if_noaccess);
+  }
+
+  Bind(&has_access);
+}
+
 // ES#sec-promise-reject-functions
 // Promise Reject Functions
 TF_BUILTIN(PromiseRejectClosure, PromiseBuiltinsAssembler) {
   Node* const value = Parameter(1);
   Node* const context = Parameter(4);
 
   Label out(this);
 
   // 3. Let alreadyResolved be F.[[AlreadyResolved]].
   int has_already_visited_slot = PromiseUtils::kAlreadyVisitedSlot;
@@ skipping 37 matching lines @@
 
   Node* const executor_map = LoadMap(executor);
   GotoUnless(IsCallableMap(executor_map), &if_notcallable);
 
   Node* const native_context = LoadNativeContext(context);
   Node* const promise_fun =
       LoadContextElement(native_context, Context::PROMISE_FUNCTION_INDEX);
   Node* const is_debug_active = IsDebugActive();
   Label if_targetisnotmodified(this),
       if_targetismodified(this, Label::kDeferred), run_executor(this),
-      debug_push(this);
+      debug_push(this), if_noaccess(this, Label::kDeferred);
+
+  BranchIfAccessCheckFailed(context, promise_fun, executor, &if_noaccess);
 
   Branch(WordEqual(promise_fun, new_target), &if_targetisnotmodified,
          &if_targetismodified);
 
   Variable var_result(this, MachineRepresentation::kTagged),
       var_reject_call(this, MachineRepresentation::kTagged),
       var_reason(this, MachineRepresentation::kTagged);
 
   Bind(&if_targetisnotmodified);
   {
@@ skipping 64 matching lines @@
   }
 
   // 2. If IsCallable(executor) is false, throw a TypeError exception.
   Bind(&if_notcallable);
   {
     Node* const message_id =
         SmiConstant(MessageTemplate::kResolverNotAFunction);
     CallRuntime(Runtime::kThrowTypeError, context, message_id, executor);
     Return(UndefinedConstant());  // Never reached.
   }
+
+  // Silently fail if the stack looks fishy.
+  Bind(&if_noaccess);
+  {
+    Node* const counter_id =
+        SmiConstant(v8::Isolate::kPromiseConstructorReturnedUndefined);
+    CallRuntime(Runtime::kCountUsage, context, counter_id);
+    Return(UndefinedConstant());
+  }
 }
 
 TF_BUILTIN(PromiseInternalConstructor, PromiseBuiltinsAssembler) {
   Node* const parent = Parameter(1);
   Node* const context = Parameter(4);
   Return(AllocateAndInitJSPromise(context, parent));
 }
 
 TF_BUILTIN(IsPromise, PromiseBuiltinsAssembler) {
   Node* const maybe_promise = Parameter(1);
@@ skipping 390 matching lines @@
 
     // 5. Return promiseCapability.[[Promise]].
     Node* const promise =
         LoadObjectField(capability, JSPromiseCapability::kPromiseOffset);
     Return(promise);
   }
 }
 
 }  // namespace internal
 }  // namespace v8