Better security checks when accessing named properties via Object.getOwnPropertyDescriptor.

Current approach returns undefined descriptor if caller is not granted v8::HAS_ACCESS.
If the caller has v8::HAS_ACCESS, for no JS accessors regular v8::GET_ACCESS check is
performed and value property of the descriptor is set to undefined if caller doesn't
have proper access.  For JS accessors both v8::GET_ACCESS and v8::SET_ACCESS are checked
and affect if getter and setter would be stored in the descriptor.

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@6592 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
This commit is contained in:
antonm@chromium.org 2011-02-02 17:44:29 +00:00
parent f0573de367
commit fed5622671
2 changed files with 176 additions and 14 deletions

View File

@ -644,6 +644,77 @@ static void GetOwnPropertyImplementation(JSObject* obj,
} }
static bool CheckAccessException(LookupResult* result,
v8::AccessType access_type) {
if (result->type() == CALLBACKS) {
Object* callback = result->GetCallbackObject();
if (callback->IsAccessorInfo()) {
AccessorInfo* info = AccessorInfo::cast(callback);
bool can_access =
(access_type == v8::ACCESS_HAS &&
(info->all_can_read() || info->all_can_write())) ||
(access_type == v8::ACCESS_GET && info->all_can_read()) ||
(access_type == v8::ACCESS_SET && info->all_can_write());
return can_access;
}
}
return false;
}
static bool CheckAccess(JSObject* obj,
String* name,
LookupResult* result,
v8::AccessType access_type) {
ASSERT(result->IsProperty());
JSObject* holder = result->holder();
JSObject* current = obj;
while (true) {
if (current->IsAccessCheckNeeded() &&
!Top::MayNamedAccess(current, name, access_type)) {
// Access check callback denied the access, but some properties
// can have a special permissions which override callbacks descision
// (currently see v8::AccessControl).
break;
}
if (current == holder) {
return true;
}
current = JSObject::cast(current->GetPrototype());
}
// API callbacks can have per callback access exceptions.
switch (result->type()) {
case CALLBACKS: {
if (CheckAccessException(result, access_type)) {
return true;
}
break;
}
case INTERCEPTOR: {
// If the object has an interceptor, try real named properties.
// Overwrite the result to fetch the correct property later.
holder->LookupRealNamedProperty(name, result);
if (result->IsProperty()) {
if (CheckAccessException(result, access_type)) {
return true;
}
}
break;
}
default:
break;
}
Top::ReportFailedAccessCheck(current, access_type);
return false;
}
// Enumerator used as indices into the array returned from GetOwnProperty // Enumerator used as indices into the array returned from GetOwnProperty
enum PropertyDescriptorIndices { enum PropertyDescriptorIndices {
IS_ACCESSOR_INDEX, IS_ACCESSOR_INDEX,
@ -746,6 +817,10 @@ static MaybeObject* Runtime_GetOwnProperty(Arguments args) {
return Heap::undefined_value(); return Heap::undefined_value();
} }
if (!CheckAccess(*obj, *name, &result, v8::ACCESS_HAS)) {
return Heap::undefined_value();
}
elms->set(ENUMERABLE_INDEX, Heap::ToBoolean(!result.IsDontEnum())); elms->set(ENUMERABLE_INDEX, Heap::ToBoolean(!result.IsDontEnum()));
elms->set(CONFIGURABLE_INDEX, Heap::ToBoolean(!result.IsDontDelete())); elms->set(CONFIGURABLE_INDEX, Heap::ToBoolean(!result.IsDontDelete()));
@ -754,16 +829,22 @@ static MaybeObject* Runtime_GetOwnProperty(Arguments args) {
if (is_js_accessor) { if (is_js_accessor) {
// __defineGetter__/__defineSetter__ callback. // __defineGetter__/__defineSetter__ callback.
FixedArray* structure = FixedArray::cast(result.GetCallbackObject());
elms->set(IS_ACCESSOR_INDEX, Heap::true_value()); elms->set(IS_ACCESSOR_INDEX, Heap::true_value());
elms->set(GETTER_INDEX, structure->get(0));
elms->set(SETTER_INDEX, structure->get(1)); FixedArray* structure = FixedArray::cast(result.GetCallbackObject());
if (CheckAccess(*obj, *name, &result, v8::ACCESS_GET)) {
elms->set(GETTER_INDEX, structure->get(0));
}
if (CheckAccess(*obj, *name, &result, v8::ACCESS_SET)) {
elms->set(SETTER_INDEX, structure->get(1));
}
} else { } else {
elms->set(IS_ACCESSOR_INDEX, Heap::false_value()); elms->set(IS_ACCESSOR_INDEX, Heap::false_value());
elms->set(WRITABLE_INDEX, Heap::ToBoolean(!result.IsReadOnly())); elms->set(WRITABLE_INDEX, Heap::ToBoolean(!result.IsReadOnly()));
PropertyAttributes attrs; PropertyAttributes attrs;
Object* value; Object* value;
// GetProperty will check access and report any violations.
{ MaybeObject* maybe_value = obj->GetProperty(*obj, &result, *name, &attrs); { MaybeObject* maybe_value = obj->GetProperty(*obj, &result, *name, &attrs);
if (!maybe_value->ToObject(&value)) return maybe_value; if (!maybe_value->ToObject(&value)) return maybe_value;
} }

View File

@ -5309,11 +5309,13 @@ TEST(DetachAndReattachGlobal) {
} }
static bool allowed_access_type[v8::ACCESS_KEYS] = { false };
static bool NamedAccessBlocker(Local<v8::Object> global, static bool NamedAccessBlocker(Local<v8::Object> global,
Local<Value> name, Local<Value> name,
v8::AccessType type, v8::AccessType type,
Local<Value> data) { Local<Value> data) {
return Context::GetCurrent()->Global()->Equals(global); return Context::GetCurrent()->Global()->Equals(global) ||
allowed_access_type[type];
} }
@ -5353,7 +5355,7 @@ static void UnreachableSetter(Local<String>, Local<Value>,
} }
THREADED_TEST(AccessControl) { TEST(AccessControl) {
v8::HandleScope handle_scope; v8::HandleScope handle_scope;
v8::Handle<v8::ObjectTemplate> global_template = v8::ObjectTemplate::New(); v8::Handle<v8::ObjectTemplate> global_template = v8::ObjectTemplate::New();
@ -5379,6 +5381,15 @@ THREADED_TEST(AccessControl) {
v8::Handle<v8::Object> global0 = context0->Global(); v8::Handle<v8::Object> global0 = context0->Global();
// Define a property with JS getter and setter.
CompileRun(
"function getter() { return 'getter'; };\n"
"function setter() { return 'setter'; }\n"
"Object.defineProperty(this, 'js_accessor_p', {get:getter, set:setter})");
Local<Value> getter = global0->Get(v8_str("getter"));
Local<Value> setter = global0->Get(v8_str("setter"));
v8::HandleScope scope1; v8::HandleScope scope1;
v8::Persistent<Context> context1 = Context::New(); v8::Persistent<Context> context1 = Context::New();
@ -5387,19 +5398,89 @@ THREADED_TEST(AccessControl) {
v8::Handle<v8::Object> global1 = context1->Global(); v8::Handle<v8::Object> global1 = context1->Global();
global1->Set(v8_str("other"), global0); global1->Set(v8_str("other"), global0);
v8::Handle<Value> value;
// Access blocked property // Access blocked property
value = CompileRun("other.blocked_prop = 1"); CompileRun("other.blocked_prop = 1");
value = CompileRun("other.blocked_prop");
CHECK(value->IsUndefined());
value = CompileRun( ExpectUndefined("other.blocked_prop");
ExpectUndefined(
"Object.getOwnPropertyDescriptor(other, 'blocked_prop')");
ExpectFalse("propertyIsEnumerable.call(other, 'blocked_prop')");
// Enable ACCESS_HAS
allowed_access_type[v8::ACCESS_HAS] = true;
ExpectUndefined("other.blocked_prop");
// ... and now we can get the descriptor...
ExpectUndefined(
"Object.getOwnPropertyDescriptor(other, 'blocked_prop').value"); "Object.getOwnPropertyDescriptor(other, 'blocked_prop').value");
CHECK(value->IsUndefined()); // ... and enumerate the property.
ExpectTrue("propertyIsEnumerable.call(other, 'blocked_prop')");
allowed_access_type[v8::ACCESS_HAS] = false;
value = CompileRun("propertyIsEnumerable.call(other, 'blocked_prop')"); CompileRun("other.js_accessor_p = 2");
CHECK(value->IsFalse());
ExpectUndefined("other.js_accessor_p");
ExpectUndefined(
"Object.getOwnPropertyDescriptor(other, 'js_accessor_p')");
// Enable ACCESS_HAS.
allowed_access_type[v8::ACCESS_HAS] = true;
ExpectUndefined("other.js_accessor_p");
ExpectUndefined(
"Object.getOwnPropertyDescriptor(other, 'js_accessor_p').get");
ExpectUndefined(
"Object.getOwnPropertyDescriptor(other, 'js_accessor_p').set");
ExpectUndefined(
"Object.getOwnPropertyDescriptor(other, 'js_accessor_p').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.js_accessor_p", "getter");
ExpectObject(
"Object.getOwnPropertyDescriptor(other, 'js_accessor_p').get", getter);
ExpectUndefined(
"Object.getOwnPropertyDescriptor(other, 'js_accessor_p').set");
ExpectUndefined(
"Object.getOwnPropertyDescriptor(other, 'js_accessor_p').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.js_accessor_p");
ExpectUndefined(
"Object.getOwnPropertyDescriptor(other, 'js_accessor_p').get");
ExpectObject(
"Object.getOwnPropertyDescriptor(other, 'js_accessor_p').set", setter);
ExpectUndefined(
"Object.getOwnPropertyDescriptor(other, 'js_accessor_p').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.js_accessor_p", "getter");
ExpectObject(
"Object.getOwnPropertyDescriptor(other, 'js_accessor_p').get", getter);
ExpectObject(
"Object.getOwnPropertyDescriptor(other, 'js_accessor_p').set", setter);
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;
v8::Handle<Value> value;
// Access accessible property // Access accessible property
value = CompileRun("other.accessible_prop = 3"); value = CompileRun("other.accessible_prop = 3");