Revision: 6728
Author: [email protected]
Date: Thu Feb 10 06:09:52 2011
Log: 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
http://code.google.com/p/v8/source/detail?r=6728
Modified:
/branches/bleeding_edge/src/api.cc
/branches/bleeding_edge/test/cctest/test-api.cc
=======================================
--- /branches/bleeding_edge/src/api.cc Thu Feb 10 04:02:36 2011
+++ /branches/bleeding_edge/src/api.cc Thu Feb 10 06:09:52 2011
@@ -2661,26 +2661,38 @@
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;
}
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Thu Feb 10 04:02:36 2011
+++ /branches/bleeding_edge/test/cctest/test-api.cc Thu Feb 10 06:09:52 2011
@@ -1649,6 +1649,23 @@
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());
+ }
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev