Skip to content

Commit 21bd456

Browse files
ajkleinCommit bot
authored andcommitted
Disallow Object.observe calls on access-checked objects
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}
1 parent d346834 commit 21bd456

5 files changed

Lines changed: 52 additions & 1 deletion

File tree

src/messages.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ class CallSite {
160160
T(ObserveCallbackFrozen, \
161161
"Object.observe cannot deliver to a frozen function object") \
162162
T(ObserveGlobalProxy, "% cannot be called on the global proxy object") \
163+
T(ObserveAccessChecked, "% cannot be called on access-checked objects") \
163164
T(ObserveInvalidAccept, \
164165
"Third argument to Object.observe must be an array of strings.") \
165166
T(ObserveNonFunction, "Object.% cannot deliver to non-function") \

src/object-observe.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,8 @@ function ObjectObserve(object, callback, acceptList) {
388388
throw MakeTypeError(kObserveNonObject, "observe", "observe");
389389
if (%IsJSGlobalProxy(object))
390390
throw MakeTypeError(kObserveGlobalProxy, "observe");
391+
if (%IsAccessCheckNeeded(object))
392+
throw MakeTypeError(kObserveAccessChecked, "observe");
391393
if (!IS_CALLABLE(callback))
392394
throw MakeTypeError(kObserveNonFunction, "observe");
393395
if (ObjectIsFrozen(callback))
@@ -616,6 +618,8 @@ function ObjectGetNotifier(object) {
616618
throw MakeTypeError(kObserveNonObject, "getNotifier", "getNotifier");
617619
if (%IsJSGlobalProxy(object))
618620
throw MakeTypeError(kObserveGlobalProxy, "getNotifier");
621+
if (%IsAccessCheckNeeded(object))
622+
throw MakeTypeError(kObserveAccessChecked, "getNotifier");
619623

620624
if (ObjectIsFrozen(object)) return null;
621625

src/runtime/runtime-object.cc

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1547,5 +1547,14 @@ RUNTIME_FUNCTION(Runtime_CreateIterResultObject) {
15471547
return *isolate->factory()->NewJSIteratorResult(value, done);
15481548
}
15491549

1550+
1551+
RUNTIME_FUNCTION(Runtime_IsAccessCheckNeeded) {
1552+
SealHandleScope shs(isolate);
1553+
DCHECK_EQ(1, args.length());
1554+
CONVERT_ARG_CHECKED(Object, object, 0);
1555+
return isolate->heap()->ToBoolean(object->IsAccessCheckNeeded());
1556+
}
1557+
1558+
15501559
} // namespace internal
15511560
} // namespace v8

src/runtime/runtime.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,8 @@ namespace internal {
482482
F(StrictEquals, 2, 1) \
483483
F(InstanceOf, 2, 1) \
484484
F(HasInPrototypeChain, 2, 1) \
485-
F(CreateIterResultObject, 2, 1)
485+
F(CreateIterResultObject, 2, 1) \
486+
F(IsAccessCheckNeeded, 1, 1)
486487

487488

488489
#define FOR_EACH_INTRINSIC_OBSERVE(F) \

test/cctest/test-object-observe.cc

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -885,3 +885,39 @@ TEST(UseCountObjectGetNotifier) {
885885
CompileRun("Object.getNotifier(obj)");
886886
CHECK_EQ(1, use_counts[v8::Isolate::kObjectObserve]);
887887
}
888+
889+
890+
static bool NamedAccessCheckAlwaysAllow(Local<v8::Object> global,
891+
Local<v8::Value> name,
892+
v8::AccessType type,
893+
Local<Value> data) {
894+
return true;
895+
}
896+
897+
898+
TEST(DisallowObserveAccessCheckedObject) {
899+
v8::Isolate* isolate = CcTest::isolate();
900+
v8::HandleScope scope(isolate);
901+
LocalContext env;
902+
v8::Local<v8::ObjectTemplate> object_template =
903+
v8::ObjectTemplate::New(isolate);
904+
object_template->SetAccessCheckCallbacks(NamedAccessCheckAlwaysAllow, NULL);
905+
env->Global()->Set(v8_str("obj"), object_template->NewInstance());
906+
v8::TryCatch try_catch(isolate);
907+
CompileRun("Object.observe(obj, function(){})");
908+
CHECK(try_catch.HasCaught());
909+
}
910+
911+
912+
TEST(DisallowGetNotifierAccessCheckedObject) {
913+
v8::Isolate* isolate = CcTest::isolate();
914+
v8::HandleScope scope(isolate);
915+
LocalContext env;
916+
v8::Local<v8::ObjectTemplate> object_template =
917+
v8::ObjectTemplate::New(isolate);
918+
object_template->SetAccessCheckCallbacks(NamedAccessCheckAlwaysAllow, NULL);
919+
env->Global()->Set(v8_str("obj"), object_template->NewInstance());
920+
v8::TryCatch try_catch(isolate);
921+
CompileRun("Object.getNotifier(obj)");
922+
CHECK(try_catch.HasCaught());
923+
}

0 commit comments

Comments
 (0)