Fix forging of object's identity hashes.

Do not do standard property lookup on hidden properties object as it might
reach Object.prototype which can be altered to forge identity hashes.
Instead do only local lookup.

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

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

src/api.cc

@@ -2661,26 +2661,38 @@ int v8::Object::GetIdentityHash() {
   ENTER_V8;
   HandleScope scope;
   i::Handle<i::JSObject> self = Utils::OpenHandle(this);
-  i::Handle<i::Object> hidden_props(i::GetHiddenProperties(self, true));
-  i::Handle<i::Object> hash_symbol = i::Factory::identity_hash_symbol();
-  i::Handle<i::Object> hash = i::GetProperty(hidden_props, hash_symbol);
-  int hash_value;
-  if (hash->IsSmi()) {
-    hash_value = i::Smi::cast(*hash)->value();
-  } else {
-    int attempts = 0;
-    do {
-      // Generate a random 32-bit hash value but limit range to fit
-      // within a smi.
-      hash_value = i::V8::Random() & i::Smi::kMaxValue;
-      attempts++;
-    } while (hash_value == 0 && attempts < 30);
-    hash_value = hash_value != 0 ? hash_value : 1;  // never return 0
-    i::SetProperty(hidden_props,
-                   hash_symbol,
-                   i::Handle<i::Object>(i::Smi::FromInt(hash_value)),
-                   static_cast<PropertyAttributes>(None));
+  i::Handle<i::Object> hidden_props_obj(i::GetHiddenProperties(self, true));
+  if (!hidden_props_obj->IsJSObject()) {
+    // We failed to create hidden properties.  That's a detached
+    // global proxy.
+    ASSERT(hidden_props_obj->IsUndefined());
+    return 0;
   }
+  i::Handle<i::JSObject> hidden_props =
+      i::Handle<i::JSObject>::cast(hidden_props_obj);
+  i::Handle<i::String> hash_symbol = i::Factory::identity_hash_symbol();
+  if (hidden_props->HasLocalProperty(*hash_symbol)) {
+    i::Handle<i::Object> hash = i::GetProperty(hidden_props, hash_symbol);
+    CHECK(!hash.is_null());
+    CHECK(hash->IsSmi());
+    return i::Smi::cast(*hash)->value();
+  }
+
+  int hash_value;
+  int attempts = 0;
+  do {
+    // Generate a random 32-bit hash value but limit range to fit
+    // within a smi.
+    hash_value = i::V8::Random() & i::Smi::kMaxValue;
+    attempts++;
+  } while (hash_value == 0 && attempts < 30);
+  hash_value = hash_value != 0 ? hash_value : 1;  // never return 0
+  CHECK(!i::SetLocalPropertyIgnoreAttributes(
+          hidden_props,
+          hash_symbol,
+          i::Handle<i::Object>(i::Smi::FromInt(hash_value)),
+          static_cast<PropertyAttributes>(None)).is_null());
+
   return hash_value;
 }
 

test/cctest/test-api.cc

@@ -1649,6 +1649,23 @@ THREADED_TEST(IdentityHash) {
   CHECK_NE(hash, hash3);
   int hash4 = obj->GetIdentityHash();
   CHECK_EQ(hash, hash4);
+
+  // Check identity hashes behaviour in the presence of JS accessors.
+  // Put a getter for 'v8::IdentityHash' on the Object's prototype:
+  {
+    CompileRun("Object.prototype['v8::IdentityHash'] = 42;\n");
+    Local<v8::Object> o1 = v8::Object::New();
+    Local<v8::Object> o2 = v8::Object::New();
+    CHECK_NE(o1->GetIdentityHash(), o2->GetIdentityHash());
+  }
+  {
+    CompileRun(
+        "function cnst() { return 42; };\n"
+        "Object.prototype.__defineGetter__('v8::IdentityHash', cnst);\n");
+    Local<v8::Object> o1 = v8::Object::New();
+    Local<v8::Object> o2 = v8::Object::New();
+    CHECK_NE(o1->GetIdentityHash(), o2->GetIdentityHash());
+  }
 }