Perform security checks before fetching the value in Object.getOwnPropertyDescriptor.

Review URL: http://codereview.chromium.org/6386022

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

src/runtime.cc

@@ -745,35 +745,31 @@ static MaybeObject* Runtime_GetOwnProperty(Arguments args) {
   if (!result.IsProperty()) {
     return Heap::undefined_value();
   }
-  if (result.type() == CALLBACKS) {
-    Object* structure = result.GetCallbackObject();
-    if (structure->IsProxy() || structure->IsAccessorInfo()) {
-      // Property that is internally implemented as a callback or
-      // an API defined callback.
-      Object* value;
-      { MaybeObject* maybe_value = obj->GetPropertyWithCallback(
-            *obj, structure, *name, result.holder());
-        if (!maybe_value->ToObject(&value)) return maybe_value;
-      }
-      elms->set(IS_ACCESSOR_INDEX, Heap::false_value());
-      elms->set(VALUE_INDEX, value);
-      elms->set(WRITABLE_INDEX, Heap::ToBoolean(!result.IsReadOnly()));
-    } else if (structure->IsFixedArray()) {
-      // __defineGetter__/__defineSetter__ callback.
-      elms->set(IS_ACCESSOR_INDEX, Heap::true_value());
-      elms->set(GETTER_INDEX, FixedArray::cast(structure)->get(0));
-      elms->set(SETTER_INDEX, FixedArray::cast(structure)->get(1));
-    } else {
-      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.
+    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));
   } else {
     elms->set(IS_ACCESSOR_INDEX, Heap::false_value());
-    elms->set(VALUE_INDEX, result.GetLazyValue());
     elms->set(WRITABLE_INDEX, Heap::ToBoolean(!result.IsReadOnly()));
+
+    PropertyAttributes attrs;
+    Object* value;
+    { MaybeObject* maybe_value = obj->GetProperty(*obj, &result, *name, &attrs);
+      if (!maybe_value->ToObject(&value)) return maybe_value;
+    }
+    elms->set(VALUE_INDEX, value);
   }
 
-  elms->set(ENUMERABLE_INDEX, Heap::ToBoolean(!result.IsDontEnum()));
-  elms->set(CONFIGURABLE_INDEX, Heap::ToBoolean(!result.IsDontDelete()));
   return *desc;
 }
 

test/cctest/test-api.cc

@@ -5373,37 +5373,45 @@ THREADED_TEST(AccessControl) {
   v8::Handle<Value> value;
 
   // Access blocked property
-  value = v8_compile("other.blocked_prop = 1")->Run();
-  value = v8_compile("other.blocked_prop")->Run();
+  value = CompileRun("other.blocked_prop = 1");
+  value = CompileRun("other.blocked_prop");
   CHECK(value->IsUndefined());
 
-  value = v8_compile("propertyIsEnumerable.call(other, 'blocked_prop')")->Run();
+  value = CompileRun(
+      "Object.getOwnPropertyDescriptor(other, 'blocked_prop').value");
+  CHECK(value->IsUndefined());
+
+  value = CompileRun("propertyIsEnumerable.call(other, 'blocked_prop')");
   CHECK(value->IsFalse());
 
   // Access accessible property
-  value = v8_compile("other.accessible_prop = 3")->Run();
+  value = CompileRun("other.accessible_prop = 3");
   CHECK(value->IsNumber());
   CHECK_EQ(3, value->Int32Value());
   CHECK_EQ(3, g_echo_value);
 
-  value = v8_compile("other.accessible_prop")->Run();
+  value = CompileRun("other.accessible_prop");
   CHECK(value->IsNumber());
   CHECK_EQ(3, value->Int32Value());
 
-  value =
-    v8_compile("propertyIsEnumerable.call(other, 'accessible_prop')")->Run();
+  value = CompileRun(
+      "Object.getOwnPropertyDescriptor(other, 'accessible_prop').value");
+  CHECK(value->IsNumber());
+  CHECK_EQ(3, value->Int32Value());
+
+  value = CompileRun("propertyIsEnumerable.call(other, 'accessible_prop')");
   CHECK(value->IsTrue());
 
   // Enumeration doesn't enumerate accessors from inaccessible objects in
   // the prototype chain even if the accessors are in themselves accessible.
-  Local<Value> result =
+  value =
       CompileRun("(function(){var obj = {'__proto__':other};"
                  "for (var p in obj)"
                  "   if (p == 'accessible_prop' || p == 'blocked_prop') {"
                  "     return false;"
                  "   }"
                  "return true;})()");
-  CHECK(result->IsTrue());
+  CHECK(value->IsTrue());
 
   context1->Exit();
   context0->Exit();