Revision: 6626
Author: [email protected]
Date: Thu Feb 3 10:09:51 2011
Log: Do proper security checks when accessing elements with
getOwnPropertyDescriptor.
This extends logic applied to regular properties to elements.
Review URL: http://codereview.chromium.org/6246055
http://code.google.com/p/v8/source/detail?r=6626
Modified:
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/test/cctest/test-api.cc
=======================================
--- /branches/bleeding_edge/src/runtime.cc Thu Feb 3 04:50:50 2011
+++ /branches/bleeding_edge/src/runtime.cc Thu Feb 3 10:09:51 2011
@@ -713,6 +713,19 @@
Top::ReportFailedAccessCheck(current, access_type);
return false;
}
+
+
+// TODO(1095): we should traverse hidden prototype hierachy as well.
+static bool CheckElementAccess(JSObject* obj,
+ uint32_t index,
+ v8::AccessType access_type) {
+ if (obj->IsAccessCheckNeeded() &&
+ !Top::MayIndexedAccess(obj, index, access_type)) {
+ return false;
+ }
+
+ return true;
+}
// Enumerator used as indices into the array returned from GetOwnProperty
@@ -757,7 +770,7 @@
// subsequent cases.
Handle<JSValue> js_value = Handle<JSValue>::cast(obj);
Handle<String> str(String::cast(js_value->value()));
- Handle<String> substr = SubString(str, index, index+1,
NOT_TENURED);
+ Handle<String> substr = SubString(str, index, index + 1,
NOT_TENURED);
elms->set(IS_ACCESSOR_INDEX, Heap::false_value());
elms->set(VALUE_INDEX, *substr);
@@ -770,8 +783,7 @@
case JSObject::INTERCEPTED_ELEMENT:
case JSObject::FAST_ELEMENT: {
elms->set(IS_ACCESSOR_INDEX, Heap::false_value());
- Handle<Object> element = GetElement(Handle<Object>(obj), index);
- elms->set(VALUE_INDEX, *element);
+ elms->set(VALUE_INDEX, *GetElement(obj, index));
elms->set(WRITABLE_INDEX, Heap::true_value());
elms->set(ENUMERABLE_INDEX, Heap::true_value());
elms->set(CONFIGURABLE_INDEX, Heap::true_value());
@@ -779,13 +791,14 @@
}
case JSObject::DICTIONARY_ELEMENT: {
+ Handle<JSObject> holder = obj;
if (obj->IsJSGlobalProxy()) {
Object* proto = obj->GetPrototype();
if (proto->IsNull()) return Heap::undefined_value();
ASSERT(proto->IsJSGlobalObject());
- obj = Handle<JSObject>(JSObject::cast(proto));
- }
- NumberDictionary* dictionary = obj->element_dictionary();
+ holder = Handle<JSObject>(JSObject::cast(proto));
+ }
+ NumberDictionary* dictionary = holder->element_dictionary();
int entry = dictionary->FindEntry(index);
ASSERT(entry != NumberDictionary::kNotFound);
PropertyDetails details = dictionary->DetailsAt(entry);
@@ -795,14 +808,18 @@
FixedArray* callbacks =
FixedArray::cast(dictionary->ValueAt(entry));
elms->set(IS_ACCESSOR_INDEX, Heap::true_value());
- elms->set(GETTER_INDEX, callbacks->get(0));
- elms->set(SETTER_INDEX, callbacks->get(1));
+ if (CheckElementAccess(*obj, index, v8::ACCESS_GET)) {
+ elms->set(GETTER_INDEX, callbacks->get(0));
+ }
+ if (CheckElementAccess(*obj, index, v8::ACCESS_SET)) {
+ elms->set(SETTER_INDEX, callbacks->get(1));
+ }
break;
}
case NORMAL:
// This is a data property.
elms->set(IS_ACCESSOR_INDEX, Heap::false_value());
- elms->set(VALUE_INDEX, dictionary->ValueAt(entry));
+ elms->set(VALUE_INDEX, *GetElement(obj, index));
elms->set(WRITABLE_INDEX,
Heap::ToBoolean(!details.IsReadOnly()));
break;
default:
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Thu Feb 3 05:30:22 2011
+++ /branches/bleeding_edge/test/cctest/test-api.cc Thu Feb 3 10:09:51 2011
@@ -5323,7 +5323,8 @@
uint32_t key,
v8::AccessType type,
Local<Value> data) {
- return Context::GetCurrent()->Global()->Equals(global);
+ return Context::GetCurrent()->Global()->Equals(global) ||
+ allowed_access_type[type];
}
@@ -5390,6 +5391,18 @@
Local<Value> getter = global0->Get(v8_str("getter"));
Local<Value> setter = global0->Get(v8_str("setter"));
+ // And define normal element.
+ global0->Set(239, v8_str("239"));
+
+ // Define an element with JS getter and setter.
+ CompileRun(
+ "function el_getter() { return 'el_getter'; };\n"
+ "function el_setter() { return 'el_setter'; };\n"
+ "Object.defineProperty(this, '42', {get: el_getter, set:
el_setter});");
+
+ Local<Value> el_getter = global0->Get(v8_str("el_getter"));
+ Local<Value> el_setter = global0->Get(v8_str("el_setter"));
+
v8::HandleScope scope1;
v8::Persistent<Context> context1 = Context::New();
@@ -5398,7 +5411,7 @@
v8::Handle<v8::Object> global1 = context1->Global();
global1->Set(v8_str("other"), global0);
- // Access blocked property
+ // Access blocked property.
CompileRun("other.blocked_prop = 1");
ExpectUndefined("other.blocked_prop");
@@ -5416,6 +5429,23 @@
ExpectTrue("propertyIsEnumerable.call(other, 'blocked_prop')");
allowed_access_type[v8::ACCESS_HAS] = false;
+ // Access blocked element.
+ CompileRun("other[239] = 1");
+
+ ExpectUndefined("other[239]");
+ ExpectUndefined("Object.getOwnPropertyDescriptor(other, '239')");
+ ExpectFalse("propertyIsEnumerable.call(other, '239')");
+
+ // Enable ACCESS_HAS
+ allowed_access_type[v8::ACCESS_HAS] = true;
+ ExpectUndefined("other[239]");
+ // ... and now we can get the descriptor...
+ ExpectUndefined("Object.getOwnPropertyDescriptor(other, '239').value");
+ // ... and enumerate the property.
+ ExpectTrue("propertyIsEnumerable.call(other, '239')");
+ allowed_access_type[v8::ACCESS_HAS] = false;
+
+ // Access a property with JS accessor.
CompileRun("other.js_accessor_p = 2");
ExpectUndefined("other.js_accessor_p");
@@ -5476,6 +5506,58 @@
ExpectUndefined(
"Object.getOwnPropertyDescriptor(other, 'js_accessor_p').value");
+ allowed_access_type[v8::ACCESS_SET] = false;
+ allowed_access_type[v8::ACCESS_GET] = false;
+ allowed_access_type[v8::ACCESS_HAS] = false;
+
+ // Access an element with JS accessor.
+ CompileRun("other[42] = 2");
+
+ ExpectUndefined("other[42]");
+ ExpectUndefined("Object.getOwnPropertyDescriptor(other, '42')");
+
+ // Enable ACCESS_HAS.
+ allowed_access_type[v8::ACCESS_HAS] = true;
+ ExpectUndefined("other[42]");
+ ExpectUndefined("Object.getOwnPropertyDescriptor(other, '42').get");
+ ExpectUndefined("Object.getOwnPropertyDescriptor(other, '42').set");
+ ExpectUndefined("Object.getOwnPropertyDescriptor(other, '42').value");
+ allowed_access_type[v8::ACCESS_HAS] = false;
+
+ // Enable both ACCESS_HAS and ACCESS_GET.
+ allowed_access_type[v8::ACCESS_HAS] = true;
+ allowed_access_type[v8::ACCESS_GET] = true;
+
+ ExpectString("other[42]", "el_getter");
+ ExpectObject("Object.getOwnPropertyDescriptor(other, '42').get",
el_getter);
+ ExpectUndefined("Object.getOwnPropertyDescriptor(other, '42').set");
+ ExpectUndefined("Object.getOwnPropertyDescriptor(other, '42').value");
+
+ allowed_access_type[v8::ACCESS_GET] = false;
+ allowed_access_type[v8::ACCESS_HAS] = false;
+
+ // Enable both ACCESS_HAS and ACCESS_SET.
+ allowed_access_type[v8::ACCESS_HAS] = true;
+ allowed_access_type[v8::ACCESS_SET] = true;
+
+ ExpectUndefined("other[42]");
+ ExpectUndefined("Object.getOwnPropertyDescriptor(other, '42').get");
+ ExpectObject("Object.getOwnPropertyDescriptor(other, '42').set",
el_setter);
+ ExpectUndefined("Object.getOwnPropertyDescriptor(other, '42').value");
+
+ allowed_access_type[v8::ACCESS_SET] = false;
+ allowed_access_type[v8::ACCESS_HAS] = false;
+
+ // Enable both ACCESS_HAS, ACCESS_GET and ACCESS_SET.
+ allowed_access_type[v8::ACCESS_HAS] = true;
+ allowed_access_type[v8::ACCESS_GET] = true;
+ allowed_access_type[v8::ACCESS_SET] = true;
+
+ ExpectString("other[42]", "el_getter");
+ ExpectObject("Object.getOwnPropertyDescriptor(other, '42').get",
el_getter);
+ ExpectObject("Object.getOwnPropertyDescriptor(other, '42').set",
el_setter);
+ ExpectUndefined("Object.getOwnPropertyDescriptor(other, '42').value");
+
allowed_access_type[v8::ACCESS_SET] = false;
allowed_access_type[v8::ACCESS_GET] = false;
allowed_access_type[v8::ACCESS_HAS] = false;
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
