Do security checks in the promise constructor
Since we only can do limited checks during microtask execution, do the checks before actually creating a promise BUG=chromium:658194 R=bmeurer@chromium.org,gsathya@chromium.org Review-Url: https://codereview.chromium.org/2628863002 Cr-Commit-Position: refs/heads/master@{#42265}
This commit is contained in:
parent
b8294aaa97
commit
81c62e070b
@ -6508,6 +6508,7 @@ class V8_EXPORT Isolate {
|
||||
kFunctionConstructorReturnedUndefined = 35,
|
||||
kAssigmentExpressionLHSIsCallInSloppy = 36,
|
||||
kAssigmentExpressionLHSIsCallInStrict = 37,
|
||||
kPromiseConstructorReturnedUndefined = 38,
|
||||
|
||||
// If you add new values here, you'll also need to update Chromium's:
|
||||
// UseCounter.h, V8PerIsolateData.cpp, histograms.xml
|
||||
|
@ -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) {
|
||||
|
@ -81,6 +81,10 @@ class PromiseBuiltinsAssembler : public CodeStubAssembler {
|
||||
Node* NewPromiseCapability(Node* context, Node* constructor,
|
||||
Node* debug_event = nullptr);
|
||||
|
||||
void BranchIfAccessCheckFailed(Node* context, Node* native_context,
|
||||
Node* promise_constructor, Node* executor,
|
||||
Label* if_noaccess);
|
||||
|
||||
protected:
|
||||
void PromiseInit(Node* promise);
|
||||
|
||||
|
@ -2872,6 +2872,10 @@ Node* CodeStubAssembler::ThrowIfNotInstanceType(Node* context, Node* value,
|
||||
return var_value_map.value();
|
||||
}
|
||||
|
||||
Node* CodeStubAssembler::InstanceTypeEqual(Node* instance_type, int type) {
|
||||
return Word32Equal(instance_type, Int32Constant(type));
|
||||
}
|
||||
|
||||
Node* CodeStubAssembler::IsSpecialReceiverMap(Node* map) {
|
||||
Node* is_special = IsSpecialReceiverInstanceType(LoadMapInstanceType(map));
|
||||
uint32_t mask =
|
||||
|
@ -667,6 +667,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
|
||||
// Type checks.
|
||||
// Check whether the map is for an object with special properties, such as a
|
||||
// JSProxy or an object with interceptors.
|
||||
Node* InstanceTypeEqual(Node* instance_type, int type);
|
||||
Node* IsSpecialReceiverMap(Node* map);
|
||||
Node* IsSpecialReceiverInstanceType(Node* instance_type);
|
||||
Node* IsStringInstanceType(Node* instance_type);
|
||||
|
@ -9,6 +9,7 @@
|
||||
#include "src/arguments.h"
|
||||
#include "src/ast/prettyprinter.h"
|
||||
#include "src/bootstrapper.h"
|
||||
#include "src/builtins/builtins.h"
|
||||
#include "src/conversions.h"
|
||||
#include "src/debug/debug.h"
|
||||
#include "src/frames-inl.h"
|
||||
@ -494,5 +495,14 @@ RUNTIME_FUNCTION(Runtime_Typeof) {
|
||||
return *Object::TypeOf(isolate, object);
|
||||
}
|
||||
|
||||
RUNTIME_FUNCTION(Runtime_AllowDynamicFunction) {
|
||||
HandleScope scope(isolate);
|
||||
DCHECK_EQ(1, args.length());
|
||||
CONVERT_ARG_HANDLE_CHECKED(JSFunction, target, 0);
|
||||
Handle<JSObject> global_proxy(target->global_proxy(), isolate);
|
||||
return *isolate->factory()->ToBoolean(
|
||||
Builtins::AllowDynamicFunction(isolate, target, global_proxy));
|
||||
}
|
||||
|
||||
} // namespace internal
|
||||
} // namespace v8
|
||||
|
@ -339,7 +339,8 @@ namespace internal {
|
||||
F(ThrowTypeError, -1 /* >= 1 */, 1) \
|
||||
F(ThrowUndefinedOrNullToObject, 1, 1) \
|
||||
F(Typeof, 1, 1) \
|
||||
F(UnwindAndFindExceptionHandler, 0, 1)
|
||||
F(UnwindAndFindExceptionHandler, 0, 1) \
|
||||
F(AllowDynamicFunction, 1, 1)
|
||||
|
||||
#define FOR_EACH_INTRINSIC_LITERALS(F) \
|
||||
F(CreateRegExpLiteral, 4, 1) \
|
||||
|
@ -96,6 +96,8 @@ var ctor_a_script =
|
||||
var ctor_b_script = "Function.bind(this, 'return 1;')";
|
||||
var ctor_c_script =
|
||||
"(function() { return Function.call(this, 'return 1;'); })";
|
||||
// Also check Promise constructor.
|
||||
var promise_ctor_script = "Promise";
|
||||
Realm.shared = {
|
||||
ctor_0 : Realm.eval(realms[0], ctor_script),
|
||||
ctor_1 : Realm.eval(realms[1], ctor_script),
|
||||
@ -105,9 +107,12 @@ Realm.shared = {
|
||||
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),
|
||||
promise_ctor_0 : Realm.eval(realms[0], promise_ctor_script),
|
||||
promise_ctor_1 : Realm.eval(realms[1], promise_ctor_script),
|
||||
}
|
||||
var script_0 = " \
|
||||
var ctor_0 = Realm.shared.ctor_0; \
|
||||
var promise_ctor_0 = Realm.shared.promise_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']); \
|
||||
@ -118,6 +123,7 @@ var script_0 = " \
|
||||
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(); \
|
||||
Realm.shared.p_0 = new promise_ctor_0((res,rej) => res(1)); \
|
||||
";
|
||||
script = script_0 + script_0.replace(/_0/g, "_1");
|
||||
Realm.eval(realms[0], script);
|
||||
@ -131,6 +137,7 @@ assertSame(1, Realm.shared.reflect_0());
|
||||
assertSame(1, Realm.shared.a_0());
|
||||
assertSame(1, Realm.shared.b_0());
|
||||
assertSame(1, Realm.shared.c_0());
|
||||
assertInstanceof(Realm.shared.p_0, Realm.shared.promise_ctor_0);
|
||||
assertSame(undefined, Realm.shared.direct_1);
|
||||
assertSame(undefined, Realm.shared.indirect_1);
|
||||
assertSame(undefined, Realm.shared.apply_1);
|
||||
@ -141,6 +148,7 @@ assertSame(undefined, Realm.shared.reflect_1);
|
||||
assertSame(undefined, Realm.shared.a_1);
|
||||
assertSame(undefined, Realm.shared.b_1);
|
||||
assertSame(undefined, Realm.shared.c_1);
|
||||
assertSame(undefined, Realm.shared.p_1);
|
||||
Realm.eval(realms[1], script);
|
||||
assertSame(undefined, Realm.shared.direct_0);
|
||||
assertSame(undefined, Realm.shared.indirect_0);
|
||||
@ -152,6 +160,7 @@ 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(undefined, Realm.shared.p_0);
|
||||
assertSame(1, Realm.shared.direct_1());
|
||||
assertSame(1, Realm.shared.indirect_1());
|
||||
assertSame(1, Realm.shared.apply_1());
|
||||
@ -162,3 +171,4 @@ assertSame(1, Realm.shared.reflect_1());
|
||||
assertSame(1, Realm.shared.a_1());
|
||||
assertSame(1, Realm.shared.b_1());
|
||||
assertSame(1, Realm.shared.c_1());
|
||||
assertInstanceof(Realm.shared.p_1, Realm.shared.promise_ctor_1);
|
||||
|
Loading…
Reference in New Issue
Block a user