From 5d3430a5094a7f55149b4d49f2cac0ce13b785c3 Mon Sep 17 00:00:00 2001
From: "antonm@chromium.org"
 <antonm@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Thu, 10 Feb 2011 14:09:52 +0000
Subject: [PATCH] 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              | 50 +++++++++++++++++++++++++----------------
 test/cctest/test-api.cc | 17 ++++++++++++++
 2 files changed, 48 insertions(+), 19 deletions(-)

diff --git a/src/api.cc b/src/api.cc
index 8d3018409a..399d091ac7 100644
--- a/src/api.cc
+++ b/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;
 }
 
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index b68fdffe19..b92185f52a 100644
--- a/test/cctest/test-api.cc
+++ b/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());
+  }
 }