Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
Disallow Object.observe calls on access-checked objects
Browse files Browse the repository at this point in the history
We already disallowed observing the global proxy; now we also
disallow any observation of access-checked objects (regardless
of whether the access check would succeed or fail, since there's
not a good way to tell the embedder what kind of access is being
requested).

Also disallow Object.getNotifier for the same reasons.

BUG=chromium:531891
LOG=y

Review URL: https://codereview.chromium.org/1346813002

Cr-Commit-Position: refs/heads/master@{#30774}
  • Loading branch information
ajklein authored and Commit bot committed Sep 16, 2015
1 parent d346834 commit 21bd456
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 1 deletion.
1 change: 1 addition & 0 deletions src/messages.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ class CallSite {
T(ObserveCallbackFrozen, \
"Object.observe cannot deliver to a frozen function object") \
T(ObserveGlobalProxy, "% cannot be called on the global proxy object") \
T(ObserveAccessChecked, "% cannot be called on access-checked objects") \
T(ObserveInvalidAccept, \
"Third argument to Object.observe must be an array of strings.") \
T(ObserveNonFunction, "Object.% cannot deliver to non-function") \
Expand Down
4 changes: 4 additions & 0 deletions src/object-observe.js
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,8 @@ function ObjectObserve(object, callback, acceptList) {
throw MakeTypeError(kObserveNonObject, "observe", "observe");
if (%IsJSGlobalProxy(object))
throw MakeTypeError(kObserveGlobalProxy, "observe");
if (%IsAccessCheckNeeded(object))
throw MakeTypeError(kObserveAccessChecked, "observe");
if (!IS_CALLABLE(callback))
throw MakeTypeError(kObserveNonFunction, "observe");
if (ObjectIsFrozen(callback))
Expand Down Expand Up @@ -616,6 +618,8 @@ function ObjectGetNotifier(object) {
throw MakeTypeError(kObserveNonObject, "getNotifier", "getNotifier");
if (%IsJSGlobalProxy(object))
throw MakeTypeError(kObserveGlobalProxy, "getNotifier");
if (%IsAccessCheckNeeded(object))
throw MakeTypeError(kObserveAccessChecked, "getNotifier");

if (ObjectIsFrozen(object)) return null;

Expand Down
9 changes: 9 additions & 0 deletions src/runtime/runtime-object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1547,5 +1547,14 @@ RUNTIME_FUNCTION(Runtime_CreateIterResultObject) {
return *isolate->factory()->NewJSIteratorResult(value, done);
}


RUNTIME_FUNCTION(Runtime_IsAccessCheckNeeded) {
SealHandleScope shs(isolate);
DCHECK_EQ(1, args.length());
CONVERT_ARG_CHECKED(Object, object, 0);
return isolate->heap()->ToBoolean(object->IsAccessCheckNeeded());
}


} // namespace internal
} // namespace v8
3 changes: 2 additions & 1 deletion src/runtime/runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,8 @@ namespace internal {
F(StrictEquals, 2, 1) \
F(InstanceOf, 2, 1) \
F(HasInPrototypeChain, 2, 1) \
F(CreateIterResultObject, 2, 1)
F(CreateIterResultObject, 2, 1) \
F(IsAccessCheckNeeded, 1, 1)


#define FOR_EACH_INTRINSIC_OBSERVE(F) \
Expand Down
36 changes: 36 additions & 0 deletions test/cctest/test-object-observe.cc
Original file line number Diff line number Diff line change
Expand Up @@ -885,3 +885,39 @@ TEST(UseCountObjectGetNotifier) {
CompileRun("Object.getNotifier(obj)");
CHECK_EQ(1, use_counts[v8::Isolate::kObjectObserve]);
}


static bool NamedAccessCheckAlwaysAllow(Local<v8::Object> global,
Local<v8::Value> name,
v8::AccessType type,
Local<Value> data) {
return true;
}


TEST(DisallowObserveAccessCheckedObject) {
v8::Isolate* isolate = CcTest::isolate();
v8::HandleScope scope(isolate);
LocalContext env;
v8::Local<v8::ObjectTemplate> object_template =
v8::ObjectTemplate::New(isolate);
object_template->SetAccessCheckCallbacks(NamedAccessCheckAlwaysAllow, NULL);
env->Global()->Set(v8_str("obj"), object_template->NewInstance());
v8::TryCatch try_catch(isolate);
CompileRun("Object.observe(obj, function(){})");
CHECK(try_catch.HasCaught());
}


TEST(DisallowGetNotifierAccessCheckedObject) {
v8::Isolate* isolate = CcTest::isolate();
v8::HandleScope scope(isolate);
LocalContext env;
v8::Local<v8::ObjectTemplate> object_template =
v8::ObjectTemplate::New(isolate);
object_template->SetAccessCheckCallbacks(NamedAccessCheckAlwaysAllow, NULL);
env->Global()->Set(v8_str("obj"), object_template->NewInstance());
v8::TryCatch try_catch(isolate);
CompileRun("Object.getNotifier(obj)");
CHECK(try_catch.HasCaught());
}

0 comments on commit 21bd456

Please sign in to comment.