Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(940)

Unified Diff: src/builtins/builtins-promise.cc

Issue 2628863002: Do security checks in the promise constructor (Closed)
Patch Set: updates Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/builtins/builtins-promise.h ('k') | src/code-stub-assembler.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/builtins/builtins-promise.cc
diff --git a/src/builtins/builtins-promise.cc b/src/builtins/builtins-promise.cc
index 345625d6d098d09eeb7686658803da7bdbcfa046..dbc655e80dcac0057ef643743f07abfa5409b3b6 100644
--- a/src/builtins/builtins-promise.cc
+++ b/src/builtins/builtins-promise.cc
@@ -904,6 +904,51 @@ void PromiseBuiltinsAssembler::PromiseFulfill(
}
}
+void PromiseBuiltinsAssembler::BranchIfAccessCheckFailed(
+ Node* context, Node* native_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);
+ {
+ Node* executor_type = LoadInstanceType(var_executor.value());
+ GotoIf(InstanceTypeEqual(executor_type, JS_FUNCTION_TYPE), &found_function);
+ GotoUnless(InstanceTypeEqual(executor_type, JS_BOUND_FUNCTION_TYPE),
+ &call_runtime);
+ 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* 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) {
@@ -961,7 +1006,10 @@ TF_BUILTIN(PromiseConstructor, PromiseBuiltinsAssembler) {
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, native_context, promise_fun, executor,
+ &if_noaccess);
Branch(WordEqual(promise_fun, new_target), &if_targetisnotmodified,
&if_targetismodified);
@@ -1046,6 +1094,15 @@ TF_BUILTIN(PromiseConstructor, PromiseBuiltinsAssembler) {
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::kIncrementUseCounter, context, counter_id);
+ Return(UndefinedConstant());
+ }
}
TF_BUILTIN(PromiseInternalConstructor, PromiseBuiltinsAssembler) {
« no previous file with comments | « src/builtins/builtins-promise.h ('k') | src/code-stub-assembler.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698