[v8-dev] Add access checks to Object.preventExtensions + add regression test for 1027.... (issue6534019)

[ricow]

Reviewers: antonm,

Description:
Add access checks to Object.preventExtensions + add regression test for  
1027.

Object.preventExtensions can currently be used cross-domain. With this
change we follow firefox (IE9 has our current behaviour). In addition
this includes a regression test for 1027 and access tests for
Object.seal and Object.freeze.



Please review this at http://codereview.chromium.org/6534019/

SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/

Affected files:
  M     src/runtime.cc
  M     test/cctest/test-api.cc


Index: src/runtime.cc
===================================================================
--- src/runtime.cc      (revision 6829)
+++ src/runtime.cc      (working copy)
@@ -881,6 +881,17 @@
 static MaybeObject* Runtime_PreventExtensions(Arguments args) {
   ASSERT(args.length() == 1);
   CONVERT_CHECKED(JSObject, obj, args[0]);
+
+  if (obj->IsJSGlobalProxy()) {
+    // If access checks fail simply return false
+    if (obj->IsAccessCheckNeeded() &&
+        !Top::MayNamedAccess(obj, Heap::undefined_value(),  
v8::ACCESS_KEYS)) {
+      Top::ReportFailedAccessCheck(obj, v8::ACCESS_KEYS);
+      return Heap::false_value();
+    }
+    obj = JSObject::cast(obj->GetPrototype());
+  }
+
   return obj->PreventExtensions();
 }

@@ -3673,6 +3684,8 @@
       is_element) {
     // Normalize the elements to enable attributes on the property.
     if (js_object->IsJSGlobalProxy()) {
+      // We do not need to do access checks here since these has already
+      // been performed by the call to GetOwnProperty.
       Handle<Object> proto(js_object->GetPrototype());
       // If proxy is detached, ignore the assignment. Alternatively,
       // we could throw an exception.
Index: test/cctest/test-api.cc
===================================================================
--- test/cctest/test-api.cc     (revision 6829)
+++ test/cctest/test-api.cc     (working copy)
@@ -5652,8 +5652,7 @@
 }


-// This is a regression test for issue 1154.
-TEST(AccessControlObjectKeys) {
+TEST(AccessControlES5) {
   v8::HandleScope handle_scope;
   v8::Handle<v8::ObjectTemplate> global_template =  
v8::ObjectTemplate::New();

@@ -5677,7 +5676,33 @@
   v8::Handle<v8::Object> global1 = context1->Global();
   global1->Set(v8_str("other"), global0);

+  // Regression test for issue 1154.
   ExpectTrue("Object.keys(other).indexOf('blocked_prop') == -1");
+
+  ExpectUndefined("other.blocked_prop");
+
+  // Regression test for issue 1027.
+  ExpectUndefined(
+      "Object.defineProperty("
+          "other,"
+          "'blocked_prop',"
+          "{configurable: false}"
+      ").blocked_prop");
+  ExpectUndefined(
+      "Object.getOwnPropertyDescriptor(other, 'blocked_prop')");
+
+  // Regression test for issue 1171.
+  ExpectTrue("Object.isExtensible(other)");
+  CompileRun("Object.preventExtensions(other)");
+  ExpectTrue("Object.isExtensible(other)");
+
+  // Seal and freeze uses other functions which already includes access
+  // checks, but we check these anyway.
+  CompileRun("Object.freeze(other)");
+  ExpectTrue("Object.isExtensible(other)");
+
+  CompileRun("Object.seal(other)");
+  ExpectTrue("Object.isExtensible(other)");
 }




--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev