Better security checks when accessing named properties via Object.getOwnPropertyDescriptor.

src/runtime.cc

 // Copyright 2010 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 626 matching lines @@
   if (!result->IsProperty()) {
     Object* proto = obj->GetPrototype();
     if (proto->IsJSObject() &&
       JSObject::cast(proto)->map()->is_hidden_prototype())
       GetOwnPropertyImplementation(JSObject::cast(proto),
                                    name, result);
   }
 }
 
 
+static bool CheckAccessException(LookupResult* result,
+                                 v8::AccessType access_type) {
+  if (result->type() == CALLBACKS) {
+    Object* callback = result->GetCallbackObject();
+    if (callback->IsAccessorInfo()) {
+      AccessorInfo* info = AccessorInfo::cast(callback);
+      bool can_access =
+          (access_type == v8::ACCESS_HAS &&
+              (info->all_can_read() || info->all_can_write())) ||
+          (access_type == v8::ACCESS_GET && info->all_can_read()) ||
+          (access_type == v8::ACCESS_SET && info->all_can_write());
+      return can_access;
+    }
+  }
+
+  return false;
+}
+
+
+static bool CheckAccess(JSObject* obj,
+                        String* name,
+                        LookupResult* result,
+                        v8::AccessType access_type) {
+  ASSERT(result->IsProperty());
+
+  JSObject* holder = result->holder();
+  JSObject* current = obj;
+  while (true) {
+    if (current->IsAccessCheckNeeded() &&
+        !Top::MayNamedAccess(current, name, access_type)) {
+      // The only hope there is an access exception.

[Mads Ager (chromium), 2011/02/02 10:13:36]

I don't understand the comment. Could you expand on it?

[antonm, 2011/02/02 12:16:31]

Sorry.  Is new wording better?

+      break;
+    }
+
+    if (current == holder) {
+      return true;
+    }
+
+    current = JSObject::cast(current->GetPrototype());
+  }
+
+  // API callbacks can have per callback access exceptions.
+  switch (result->type()) {
+    case CALLBACKS: {
+      if (CheckAccessException(result, access_type)) {
+        return true;
+      }
+      break;
+    }
+    case INTERCEPTOR: {
+      // If the object has an interceptor, try real named properties.
+      // Overwrite the result to fetch the correct property later.
+      holder->LookupRealNamedProperty(name, result);
+      if (result->IsProperty()) {
+        if (CheckAccessException(result, access_type)) {
+          return true;
+        }
+      }
+      break;
+    }
+    default:
+      break;
+  }
+
+  Top::ReportFailedAccessCheck(current, access_type);
+  return false;
+}
+
+
 // Enumerator used as indices into the array returned from GetOwnProperty
 enum PropertyDescriptorIndices {
   IS_ACCESSOR_INDEX,
   VALUE_INDEX,
   GETTER_INDEX,
   SETTER_INDEX,
   WRITABLE_INDEX,
   ENUMERABLE_INDEX,
   CONFIGURABLE_INDEX,
   DESCRIPTOR_SIZE
@@ skipping 82 matching lines @@
     }
   }
 
   // Use recursive implementation to also traverse hidden prototypes
   GetOwnPropertyImplementation(*obj, *name, &result);
 
   if (!result.IsProperty()) {
     return Heap::undefined_value();
   }
 
+  if (!CheckAccess(*obj, *name, &result, v8::ACCESS_HAS)) {
+    return Heap::undefined_value();
+  }
+
   elms->set(ENUMERABLE_INDEX, Heap::ToBoolean(!result.IsDontEnum()));
   elms->set(CONFIGURABLE_INDEX, Heap::ToBoolean(!result.IsDontDelete()));
 
   bool is_js_accessor = (result.type() == CALLBACKS) &&
                         (result.GetCallbackObject()->IsFixedArray());
 
   if (is_js_accessor) {
     // __defineGetter__/__defineSetter__ callback.
+    elms->set(IS_ACCESSOR_INDEX, Heap::true_value());
+
     FixedArray* structure = FixedArray::cast(result.GetCallbackObject());
-    elms->set(IS_ACCESSOR_INDEX, Heap::true_value());
-    elms->set(GETTER_INDEX, structure->get(0));
-    elms->set(SETTER_INDEX, structure->get(1));
+    if (CheckAccess(*obj, *name, &result, v8::ACCESS_GET)) {
+      elms->set(GETTER_INDEX, structure->get(0));
+    }
+    if (CheckAccess(*obj, *name, &result, v8::ACCESS_SET)) {
+      elms->set(SETTER_INDEX, structure->get(1));
+    }
   } else {
     elms->set(IS_ACCESSOR_INDEX, Heap::false_value());
     elms->set(WRITABLE_INDEX, Heap::ToBoolean(!result.IsReadOnly()));
 
     PropertyAttributes attrs;
     Object* value;
+    // GetProperty will check access and report any violations.
     { MaybeObject* maybe_value = obj->GetProperty(*obj, &result, *name, &attrs);
       if (!maybe_value->ToObject(&value)) return maybe_value;
     }
     elms->set(VALUE_INDEX, value);
   }
 
   return *desc;
 }
 
 
@@ skipping 10093 matching lines @@
   } else {
     // Handle last resort GC and make sure to allow future allocations
     // to grow the heap without causing GCs (if possible).
     Counters::gc_last_resort_from_js.Increment();
     Heap::CollectAllGarbage(false);
   }
 }
 
 
 } }  // namespace v8::internal