Add access check for observed objects

This change is mostly straightforward: for 'normal' sorts of change records,
simply don't deliver a changeRecord to a given observer callback if an access
the callback's Context is not allowed to "GET" or "HAS" changeRecord.name on
changeRecord.object, or if ACCESS_KEYS is disallowed.

For 'splice' records, the question of whether to hand it to an observer is trickier, since
there are multiple properties involved, and multiple types of possible information leakage.
Given that access-checked objects are very rare (only two in Blink, Window and Location),
and that they are not normally used as Arrays, it seems better to simply not emit any splice
records for such objects rather than spending lots of logic to attempt to avoid information
leakage for something that may never happen.

BUG=v8:2778
R=rossberg@chromium.org

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@16663 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/object-observe.js

@@ -367,13 +367,22 @@ function ArrayUnobserve(object, callback) {
   return ObjectUnobserve(object, callback);
 }
 
-function ObserverEnqueueIfActive(observer, objectInfo, changeRecord) {
+function ObserverEnqueueIfActive(observer, objectInfo, changeRecord,
+                                 needsAccessCheck) {
   if (!ObserverIsActive(observer, objectInfo) ||
       !TypeMapHasType(ObserverGetAcceptTypes(observer), changeRecord.type)) {
     return;
   }
 
   var callback = ObserverGetCallback(observer);
+  if (needsAccessCheck &&
+      // Drop all splice records on the floor for access-checked objects
+      (changeRecord.type == 'splice' ||
+       !%IsAccessAllowedForObserver(
+           callback, changeRecord.object, changeRecord.name))) {
+    return;
+  }
+
   var callbackInfo = CallbackInfoNormalize(callback);
   if (!observationState.pendingObservers)
     observationState.pendingObservers = { __proto__: null };
@@ -382,19 +391,25 @@ function ObserverEnqueueIfActive(observer, objectInfo, changeRecord) {
   %SetObserverDeliveryPending();
 }
 
-function ObjectInfoEnqueueChangeRecord(objectInfo, changeRecord) {
+function ObjectInfoEnqueueChangeRecord(objectInfo, changeRecord,
+                                       skipAccessCheck) {
   // TODO(rossberg): adjust once there is a story for symbols vs proxies.
   if (IS_SYMBOL(changeRecord.name)) return;
 
+  var needsAccessCheck = !skipAccessCheck &&
+      %IsAccessCheckNeeded(changeRecord.object);
+
   if (ChangeObserversIsOptimized(objectInfo.changeObservers)) {
     var observer = objectInfo.changeObservers;
-    ObserverEnqueueIfActive(observer, objectInfo, changeRecord);
+    ObserverEnqueueIfActive(observer, objectInfo, changeRecord,
+                            needsAccessCheck);
     return;
   }
 
   for (var priority in objectInfo.changeObservers) {
     var observer = objectInfo.changeObservers[priority];
-    ObserverEnqueueIfActive(observer, objectInfo, changeRecord);
+    ObserverEnqueueIfActive(observer, objectInfo, changeRecord,
+                            needsAccessCheck);
   }
 }
 
@@ -463,7 +478,8 @@ function ObjectNotifierNotify(changeRecord) {
   }
   ObjectFreeze(newRecord);
 
-  ObjectInfoEnqueueChangeRecord(objectInfo, newRecord);
+  ObjectInfoEnqueueChangeRecord(objectInfo, newRecord,
+                                true /* skip access check */);
 }
 
 function ObjectNotifierPerformChange(changeType, changeFn) {

src/runtime.cc

@@ -14505,6 +14505,14 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_HaveSameMap) {
 }
 
 
+RUNTIME_FUNCTION(MaybeObject*, Runtime_IsAccessCheckNeeded) {
+  SealHandleScope shs(isolate);
+  ASSERT(args.length() == 1);
+  CONVERT_ARG_CHECKED(HeapObject, obj, 0);
+  return isolate->heap()->ToBoolean(obj->IsAccessCheckNeeded());
+}
+
+
 RUNTIME_FUNCTION(MaybeObject*, Runtime_IsObserved) {
   SealHandleScope shs(isolate);
   ASSERT(args.length() == 1);
@@ -14582,6 +14590,34 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_UnwrapGlobalProxy) {
 }
 
 
+RUNTIME_FUNCTION(MaybeObject*, Runtime_IsAccessAllowedForObserver) {
+  HandleScope scope(isolate);
+  ASSERT(args.length() == 3);
+  CONVERT_ARG_HANDLE_CHECKED(JSFunction, observer, 0);
+  CONVERT_ARG_HANDLE_CHECKED(JSObject, object, 1);
+  ASSERT(object->IsAccessCheckNeeded());
+  Handle<Object> key = args.at<Object>(2);
+  SaveContext save(isolate);
+  isolate->set_context(observer->context());
+  if (!isolate->MayNamedAccess(*object, isolate->heap()->undefined_value(),
+                               v8::ACCESS_KEYS)) {
+    return isolate->heap()->false_value();
+  }
+  bool access_allowed = false;
+  uint32_t index = 0;
+  if (key->ToArrayIndex(&index) ||
+      (key->IsString() && String::cast(*key)->AsArrayIndex(&index))) {
+    access_allowed =
+        isolate->MayIndexedAccess(*object, index, v8::ACCESS_GET) &&
+        isolate->MayIndexedAccess(*object, index, v8::ACCESS_HAS);
+  } else {
+    access_allowed = isolate->MayNamedAccess(*object, *key, v8::ACCESS_GET) &&
+        isolate->MayNamedAccess(*object, *key, v8::ACCESS_HAS);
+  }
+  return isolate->heap()->ToBoolean(access_allowed);
+}
+
+
 static MaybeObject* ArrayConstructorCommon(Isolate* isolate,
                                            Handle<JSFunction> constructor,
                                            Handle<Object> type_info,

src/runtime.h

@@ -358,6 +358,7 @@ namespace internal {
   F(GetObservationState, 0, 1) \
   F(ObservationWeakMapCreate, 0, 1) \
   F(UnwrapGlobalProxy, 1, 1) \
+  F(IsAccessAllowedForObserver, 3, 1) \
   \
   /* Harmony typed arrays */ \
   F(ArrayBufferInitialize, 2, 1)\
@@ -469,7 +470,8 @@ namespace internal {
   F(HasExternalDoubleElements, 1, 1) \
   F(HasFastProperties, 1, 1) \
   F(TransitionElementsKind, 2, 1) \
-  F(HaveSameMap, 2, 1)
+  F(HaveSameMap, 2, 1) \
+  F(IsAccessCheckNeeded, 1, 1)
 
 
 #ifdef ENABLE_DEBUGGER_SUPPORT