Add support for API accessors that prohibit overwriting by accessors

defined in JavaScript code by using __defineGetter__ and
__defineSetter__.

Also, disable access checks when configuring objects created from
templates.
Review URL: http://codereview.chromium.org/8914

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

include/v8.h

@@ -1301,11 +1301,18 @@ typedef Handle<Array> (*IndexedPropertyEnumerator)(const AccessorInfo& info);
  * Some accessors should be accessible across contexts.  These
  * accessors have an explicit access control parameter which specifies
  * the kind of cross-context access that should be allowed.
+ *
+ * Additionally, for security, accessors can prohibit overwriting by
+ * accessors defined in JavaScript.  For objects that have such
+ * accessors either locally or in their prototype chain it is not
+ * possible to overwrite the accessor by using __defineGetter__ or
+ * __defineSetter__ from JavaScript code.
  */
 enum AccessControl {
-  DEFAULT         = 0,
-  ALL_CAN_READ    = 1,
-  ALL_CAN_WRITE   = 2
+  DEFAULT               = 0,
+  ALL_CAN_READ          = 1,
+  ALL_CAN_WRITE         = 1 << 1,
+  PROHIBITS_OVERWRITING = 1 << 2
 };
 
 

src/api.cc

@@ -701,6 +701,7 @@ void FunctionTemplate::AddInstancePropertyAccessor(
   obj->set_name(*Utils::OpenHandle(*name));
   if (settings & ALL_CAN_READ) obj->set_all_can_read(true);
   if (settings & ALL_CAN_WRITE) obj->set_all_can_write(true);
+  if (settings & PROHIBITS_OVERWRITING) obj->set_prohibits_overwriting(true);
   obj->set_property_attributes(static_cast<PropertyAttributes>(attributes));
 
   i::Handle<i::Object> list(Utils::OpenHandle(this)->property_accessors());
@@ -1915,7 +1916,7 @@ void v8::Object::TurnOnAccessCheck() {
 
   i::Handle<i::Map> new_map =
     i::Factory::CopyMapDropTransitions(i::Handle<i::Map>(obj->map()));
-  new_map->set_is_access_check_needed();
+  new_map->set_is_access_check_needed(true);
   obj->set_map(*new_map);
 }
 

src/apinatives.js

@@ -82,12 +82,18 @@ function InstantiateFunction(data, name) {
 function ConfigureTemplateInstance(obj, data) {
   var properties = %GetTemplateField(data, kApiPropertyListOffset);
   if (properties) {
-    for (var i = 0; i < properties[0]; i += 3) {
-      var name = properties[i + 1];
-      var prop_data = properties[i + 2];
-      var attributes = properties[i + 3];
-      var value = Instantiate(prop_data, name);
-      %SetProperty(obj, name, value, attributes);
+    // Disable access checks while instantiating the object.
+    var requires_access_checks = %DisableAccessChecks(obj);
+    try {
+      for (var i = 0; i < properties[0]; i += 3) {
+        var name = properties[i + 1];
+        var prop_data = properties[i + 2];
+        var attributes = properties[i + 3];
+        var value = Instantiate(prop_data, name);
+        %SetProperty(obj, name, value, attributes);
+      }
+    } finally {
+      if (requires_access_checks) %EnableAccessChecks(obj);
     }
   }
 }

src/bootstrapper.cc

@@ -597,7 +597,7 @@ void Genesis::CreateRoots(v8::Handle<v8::ObjectTemplate> global_template,
 
       Handle<String> global_name = Factory::LookupAsciiSymbol("global");
       global_proxy_function->shared()->set_instance_class_name(*global_name);
-      global_proxy_function->initial_map()->set_is_access_check_needed();
+      global_proxy_function->initial_map()->set_is_access_check_needed(true);
 
       // Set global_proxy.__proto__ to js_global after ConfigureGlobalObjects
 

src/factory.cc

@@ -725,7 +725,7 @@ Handle<JSFunction> Factory::CreateApiFunction(
 
   // Mark as needs_access_check if needed.
   if (obj->needs_access_check()) {
-    map->set_is_access_check_needed();
+    map->set_is_access_check_needed(true);
   }
 
   // Set interceptor information in the map.

src/objects-inl.h

@@ -1677,6 +1677,20 @@ bool Map::has_non_instance_prototype() {
 }
 
 
+void Map::set_is_access_check_needed(bool access_check_needed) {
+  if (access_check_needed) {
+    set_bit_field(bit_field() | (1 << kIsAccessCheckNeeded));
+  } else {
+    set_bit_field(bit_field() & ~(1 << kIsAccessCheckNeeded));
+  }
+}
+
+
+bool Map::is_access_check_needed() {
+  return ((1 << kIsAccessCheckNeeded) & bit_field()) != 0;
+}
+
+
 Code::Flags Code::flags() {
   return static_cast<Flags>(READ_INT_FIELD(this, kFlagsOffset));
 }
@@ -2298,6 +2312,16 @@ void AccessorInfo::set_all_can_write(bool value) {
 }
 
 
+bool AccessorInfo::prohibits_overwriting() {
+  return BooleanBit::get(flag(), kProhibitsOverwritingBit);
+}
+
+
+void AccessorInfo::set_prohibits_overwriting(bool value) {
+  set_flag(BooleanBit::set(flag(), kProhibitsOverwritingBit, value));
+}
+
+
 PropertyAttributes AccessorInfo::property_attributes() {
   return AttributesField::decode(static_cast<uint32_t>(flag()->value()));
 }

src/objects.cc

@@ -2257,9 +2257,19 @@ void JSObject::Lookup(String* name, LookupResult* result) {
        current != Heap::null_value();
        current = JSObject::cast(current)->GetPrototype()) {
     JSObject::cast(current)->LocalLookup(name, result);
-    if (result->IsValid() && !result->IsTransitionType()) {
-      return;
-    }
+    if (result->IsValid() && !result->IsTransitionType()) return;
+  }
+  result->NotFound();
+}
+
+
+// Search object and it's prototype chain for callback properties.
+void JSObject::LookupCallback(String* name, LookupResult* result) {
+  for (Object* current = this;
+       current != Heap::null_value();
+       current = JSObject::cast(current)->GetPrototype()) {
+    JSObject::cast(current)->LocalLookupRealNamedProperty(name, result);
+    if (result->IsValid() && result->type() == CALLBACKS) return;
   }
   result->NotFound();
 }
@@ -2285,6 +2295,22 @@ Object* JSObject::DefineGetterSetter(String* name,
   uint32_t index;
   if (name->AsArrayIndex(&index)) return Heap::undefined_value();
 
+  // Check if there is an API defined callback object which prohibits
+  // callback overwriting in this object or it's prototype chain.
+  // This mechanism is needed for instance in a browser setting, where
+  // certain accessors such as window.location should not be allowed
+  // to be overwriten because allowing overwriting could potentially
+  // cause security problems.
+  LookupResult callback_result;
+  LookupCallback(name, &callback_result);
+  if (callback_result.IsValid()) {
+    Object* obj = callback_result.GetCallbackObject();
+    if (obj->IsAccessorInfo() &&
+        AccessorInfo::cast(obj)->prohibits_overwriting()) {
+      return Heap::undefined_value();
+    }
+  }
+
   // Lookup the name.
   LookupResult result;
   LocalLookup(name, &result);

src/objects.h

@@ -1280,6 +1280,7 @@ class JSObject: public HeapObject {
   void LookupRealNamedProperty(String* name, LookupResult* result);
   void LookupRealNamedPropertyInPrototypes(String* name, LookupResult* result);
   void LookupCallbackSetterInPrototypes(String* name, LookupResult* result);
+  void LookupCallback(String* name, LookupResult* result);
 
   // Returns the number of properties on this object filtering out properties
   // with the specified attributes (ignoring interceptors).
@@ -2364,13 +2365,8 @@ class Map: public HeapObject {
 
   // Tells whether the instance needs security checks when accessing its
   // properties.
-  inline void set_is_access_check_needed() {
-    set_bit_field(bit_field() | (1 << kIsAccessCheckNeeded));
-  }
-
-  inline bool is_access_check_needed() {
-    return ((1 << kIsAccessCheckNeeded) & bit_field()) != 0;
-  }
+  inline void set_is_access_check_needed(bool access_check_needed);
+  inline bool is_access_check_needed();
 
   // [prototype]: implicit prototype object.
   DECL_ACCESSORS(prototype, Object)
@@ -3717,6 +3713,9 @@ class AccessorInfo: public Struct {
   inline bool all_can_write();
   inline void set_all_can_write(bool value);
 
+  inline bool prohibits_overwriting();
+  inline void set_prohibits_overwriting(bool value);
+
   inline PropertyAttributes property_attributes();
   inline void set_property_attributes(PropertyAttributes attributes);
 
@@ -3736,9 +3735,10 @@ class AccessorInfo: public Struct {
 
  private:
   // Bit positions in flag.
-  static const int kAllCanReadBit  = 0;
+  static const int kAllCanReadBit = 0;
   static const int kAllCanWriteBit = 1;
-  class AttributesField: public BitField<PropertyAttributes, 2, 3> {};
+  static const int kProhibitsOverwritingBit = 2;
+  class AttributesField: public BitField<PropertyAttributes, 3, 3> {};
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(AccessorInfo);
 };

src/runtime.cc

@@ -336,6 +336,23 @@ static Object* Runtime_GetTemplateField(Arguments args) {
 }
 
 
+static Object* Runtime_DisableAccessChecks(Arguments args) {
+  ASSERT(args.length() == 1);
+  CONVERT_CHECKED(HeapObject, object, args[0]);
+  bool needs_access_checks = object->map()->is_access_check_needed();
+  object->map()->set_is_access_check_needed(false);
+  return needs_access_checks ? Heap::true_value() : Heap::false_value();
+}
+
+
+static Object* Runtime_EnableAccessChecks(Arguments args) {
+  ASSERT(args.length() == 1);
+  CONVERT_CHECKED(HeapObject, object, args[0]);
+  object->map()->set_is_access_check_needed(true);
+  return Heap::undefined_value();
+}
+
+
 static Object* ThrowRedeclarationError(const char* type, Handle<String> name) {
   HandleScope scope;
   Handle<Object> type_handle = Factory::NewStringFromAscii(CStrVector(type));

src/runtime.h

@@ -179,6 +179,8 @@ namespace v8 { namespace internal {
   F(CreateApiFunction, 1) \
   F(IsTemplate, 1) \
   F(GetTemplateField, 2) \
+  F(DisableAccessChecks, 1) \
+  F(EnableAccessChecks, 1) \
   \
   /* Dates */ \
   F(DateCurrentTime, 0) \

test/cctest/test-api.cc

@@ -5075,3 +5075,80 @@ THREADED_TEST(PropertyEnumeration) {
   const char* elmv3[] = {"w", "z", "x", "y"};
   CheckProperties(elms->Get(v8::Integer::New(3)), elmc3, elmv3);
 }
+
+
+static v8::Handle<Value> AccessorProhibitsOverwritingGetter(
+    Local<String> name,
+    const AccessorInfo& info) {
+  ApiTestFuzzer::Fuzz();
+  return v8::True();
+}
+
+
+THREADED_TEST(AccessorProhibitsOverwriting) {
+  v8::HandleScope scope;
+  LocalContext context;
+  Local<ObjectTemplate> templ = ObjectTemplate::New();
+  templ->SetAccessor(v8_str("x"),
+                     AccessorProhibitsOverwritingGetter,
+                     0,
+                     v8::Handle<Value>(),
+                     v8::PROHIBITS_OVERWRITING,
+                     v8::ReadOnly);
+  Local<v8::Object> instance = templ->NewInstance();
+  context->Global()->Set(v8_str("obj"), instance);
+  Local<Value> value = CompileRun(
+      "obj.__defineGetter__('x', function() { return false; });"
+      "obj.x");
+  CHECK(value->BooleanValue());
+  value = CompileRun(
+      "var setter_called = false;"
+      "obj.__defineSetter__('x', function() { setter_called = true; });"
+      "obj.x = 42;"
+      "setter_called");
+  CHECK(!value->BooleanValue());
+  value = CompileRun(
+      "obj2 = {};"
+      "obj2.__proto__ = obj;"
+      "obj2.__defineGetter__('x', function() { return false; });"
+      "obj2.x");
+  CHECK(value->BooleanValue());
+  value = CompileRun(
+      "var setter_called = false;"
+      "obj2 = {};"
+      "obj2.__proto__ = obj;"
+      "obj2.__defineSetter__('x', function() { setter_called = true; });"
+      "obj2.x = 42;"
+      "setter_called");
+  CHECK(!value->BooleanValue());
+}
+
+
+static bool NamedSetAccessBlocker(Local<v8::Object> obj,
+                                  Local<Value> name,
+                                  v8::AccessType type,
+                                  Local<Value> data) {
+  return type != v8::ACCESS_SET;
+}
+
+
+static bool IndexedSetAccessBlocker(Local<v8::Object> obj,
+                                    uint32_t key,
+                                    v8::AccessType type,
+                                    Local<Value> data) {
+  return type != v8::ACCESS_SET;
+}
+
+
+THREADED_TEST(DisableAccessChecksWhileConfiguring) {
+  v8::HandleScope scope;
+  LocalContext context;
+  Local<ObjectTemplate> templ = ObjectTemplate::New();
+  templ->SetAccessCheckCallbacks(NamedSetAccessBlocker,
+                                 IndexedSetAccessBlocker);
+  templ->Set(v8_str("x"), v8::True());
+  Local<v8::Object> instance = templ->NewInstance();
+  context->Global()->Set(v8_str("obj"), instance);
+  Local<Value> value = CompileRun("obj.x");
+  CHECK(value->BooleanValue());
+}